standardrb / standard

Ruby's bikeshed-proof linter and formatter 🚲
Other
2.71k stars 211 forks source link

Question about `Layout/RescueEnsureAlignment` #193

Closed bruno- closed 4 years ago

bruno- commented 4 years ago

Hi,

I'm a happy standardrb user, thanks for working on it. I have a question about Layout/RescueEnsureAlignment and this example piece of code:

https://github.com/socketry/async-await/blob/master/examples/chickens.rb

The example code above clashes with the mentioned rule. While I agree with all the examples for the rule I feel the rule did not take this syntax (prefixing def) into consideration.

What are your thoughts on the provided example code? Is this something that should be brought up as an issue to rubocop?

Thanks

searls commented 4 years ago

I'm unsure what the linked code is referring to because it doesn't contain rescue or ensure. To pull the example into the thread for clarity, here is the file listing:

require_relative '../lib/async/await'

class Coop
    include Async::Await

    async def count_chickens(area_name)
        3.times do |i|
            sleep rand

            puts "Found a chicken in the #{area_name}!"
        end
    end

    async def find_chicken(areas)
        puts "Searching for chicken..."

        sleep rand * 5

        return areas.sample
    end

    async def count_all_chickens
        # These methods all run at the same time.
        count_chickens("garden")
        count_chickens("house")
        count_chickens("tree")

        # Wait for all previous async work to complete...
        barrier!

        puts "There was a chicken in the #{find_chicken(["garden", "house", "tree"]).wait}"
    end
end

coop = Coop.new
coop.count_all_chickens

And here is it run with standard:

$ standardrb foo.rb 
standard: Use Ruby Standard Style (https://github.com/testdouble/standard)
standard: Run `standardrb --fix` to automatically fix some problems.
  foo.rb:1:18: Style/StringLiterals: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
  foo.rb:4:1: Layout/IndentationStyle: Tab detected in indentation.
  foo.rb:4:1: Layout/IndentationWidth: Use 2 (not 1) spaces for indentation.
  foo.rb:6:1: Layout/IndentationStyle: Tab detected in indentation.
  foo.rb:6:1: Layout/IndentationWidth: Use 2 (not 1) spaces for indentation.
  foo.rb:7:1: Layout/IndentationStyle: Tab detected in indentation.
  foo.rb:7:2: Layout/IndentationWidth: Use 2 (not 1) spaces for indentation.
  foo.rb:8:1: Layout/IndentationStyle: Tab detected in indentation.
  foo.rb:8:3: Layout/IndentationWidth: Use 2 (not 1) spaces for indentation.
  foo.rb:10:1: Layout/IndentationStyle: Tab detected in indentation.
  foo.rb:11:1: Layout/IndentationStyle: Tab detected in indentation.
  foo.rb:12:1: Layout/IndentationStyle: Tab detected in indentation.
  foo.rb:14:1: Layout/IndentationStyle: Tab detected in indentation.
  foo.rb:14:1: Layout/IndentationWidth: Use 2 (not 1) spaces for indentation.
  foo.rb:15:1: Layout/IndentationStyle: Tab detected in indentation.
  foo.rb:15:2: Layout/IndentationWidth: Use 2 (not 1) spaces for indentation.
  foo.rb:17:1: Layout/IndentationStyle: Tab detected in indentation.
  foo.rb:19:1: Layout/IndentationStyle: Tab detected in indentation.
  foo.rb:19:3: Style/RedundantReturn: Redundant `return` detected.
  foo.rb:20:1: Layout/IndentationStyle: Tab detected in indentation.
  foo.rb:22:1: Layout/IndentationStyle: Tab detected in indentation.
  foo.rb:22:1: Layout/IndentationWidth: Use 2 (not 1) spaces for indentation.
  foo.rb:23:1: Layout/IndentationStyle: Tab detected in indentation.
  foo.rb:24:1: Layout/IndentationStyle: Tab detected in indentation.
  foo.rb:24:2: Layout/IndentationWidth: Use 2 (not 1) spaces for indentation.
  foo.rb:25:1: Layout/IndentationStyle: Tab detected in indentation.
  foo.rb:26:1: Layout/IndentationStyle: Tab detected in indentation.
  foo.rb:28:1: Layout/IndentationStyle: Tab detected in indentation.
  foo.rb:29:1: Layout/IndentationStyle: Tab detected in indentation.
  foo.rb:31:1: Layout/IndentationStyle: Tab detected in indentation.
  foo.rb:32:1: Layout/IndentationStyle: Tab detected in indentation.

Notice: Disagree with these rules? While StandardRB is pre-1.0.0, feel free to submit suggestions to:
  https://github.com/testdouble/standard/issues/new

And standardrb --fix exits cleanly:

$ standardrb --fix foo.rb 
$

Resulting in:

require_relative "../lib/async/await"

class Coop
  include Async::Await

  async def count_chickens(area_name)
    3.times do |i|
      sleep rand

      puts "Found a chicken in the #{area_name}!"
    end
  end

  async def find_chicken(areas)
    puts "Searching for chicken..."

    sleep rand * 5

    areas.sample
  end

  async def count_all_chickens
    # These methods all run at the same time.
    count_chickens("garden")
    count_chickens("house")
    count_chickens("tree")

    # Wait for all previous async work to complete...
    barrier!

    puts "There was a chicken in the #{find_chicken(["garden", "house", "tree"]).wait}"
  end
end

coop = Coop.new
coop.count_all_chickens
searls commented 4 years ago

If you get a chance, let me know what I'm not seeing here / understanding about the issue -- will reopen then!

bruno- commented 4 years ago

@searls sorry, my badly describing the problem.

Here's the code that will error with standard:

class Klass
  include Async::Await

  async def some_method
    raise "foo"
  rescue
    puts "raised foo"
  end
end

Output I'm getting:

standard: Use Ruby Standard Style (https://github.com/testdouble/standard)
standard: Run `standardrb --fix` to automatically fix some problems.
  tmp/standard_error.rb:4:3: Layout/RescueEnsureAlignment: `rescue` at 4, 2 is not aligned with `def some_method` at 2, 8.

Notice: Disagree with these rules? While StandardRB is pre-1.0.0, feel free to submit suggestions to:
  https://github.com/testdouble/standard/issues/new

The reason why I originally linked to the chicken/coop example was to show that async can legitimately be used before def with somewhat popular libraries (async - maintained by ruby core committer).

What are your thoughts? Is this something that should be brought up as an issue to rubocop and not standardrb?