Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

EmptyLinesAroundAccessModifier: autocorrect hangs on a function that looks like a modifier #1484

Closed
dblock opened this issue Dec 9, 2014 · 8 comments · Fixed by #1491
Closed

Comments

@dblock
Copy link
Contributor

dblock commented Dec 9, 2014

class Hello
  def public
    true
  end

  def foo?
    public
  end
end

Tries to align public back and forth. Still working on a spec.

@dblock dblock changed the title RuboCop hangs on a function that looks like a modifier RuboCop autocorrect hangs on a function that looks like a modifier Dec 9, 2014
@dblock dblock changed the title RuboCop autocorrect hangs on a function that looks like a modifier EmptyLinesAroundAccessModifier: autocorrect hangs on a function that looks like a modifier Dec 9, 2014
@dblock
Copy link
Contributor Author

dblock commented Dec 9, 2014

Spec: dblock@01c4b7d

TBH I don't see how to fix it, but in this case AccessModifierNode#modifier_node? is returning the wrong thing. One idea could be to walk the parent tree and make sure we're not in a :def, but seems iffy to me. Ideas?

@jonas054
Copy link
Collaborator

Checking that the parent is a :class or :module node seems like the right thing to do. Or normally the grandparent as there will be a :begin node in-between if the modifier isn't the only member of the class/module.

@dblock
Copy link
Contributor Author

dblock commented Dec 11, 2014

Made a bit of progress in dblock@565b603, only one spec to go.

  1) RuboCop::Cop::Style::AccessModifierIndentation when EnforcedStyle is set to outdent auto-corrects private in complicated case
     Failure/Error: expect(corrected).to eq(['class Hello',

       expected: "class Hello\n  def foo\n    'hi'\n  end\n\n  def bar\n    Module.new do\n\n    private\n\n      def hi\n        'bye'\n      end\n    end\n  end\nend"
            got: "class Hello\n  def foo\n    'hi'\n  end\n\n  def bar\n    Module.new do\n\n     private\n\n      def hi\n        'bye'\n      end\n    end\n  end\nend"

       (compared using ==)

       Diff:
       @@ -6,7 +6,7 @@
          def bar
            Module.new do

       -    private
       +     private

              def hi
                'bye'

This one is harder, we have a block that creates a Module (or a class). The AST is like this:

(block
  (send
    (const nil :Module) :new)
  (args)
  (begin
    (send nil :private)
    (def :hi
      (args)
      (str "bye"))))

Is there already a way to identify things like these before I reinvent the wheel?

@dyatlov
Copy link

dyatlov commented Dec 11, 2014

Running rubocop -a on a code bellow and it hangs. Private goes up and down.. Body beginning and accessor spaces are conflicting

# private methods
module AnswerPrivateMethods
  extend ActiveSupport::Concern

  included do
    private

    def some_method
        puts "boo"
    end
  end
end

@dblock
Copy link
Contributor Author

dblock commented Dec 11, 2014

Yes @dyatlov that's what I am trying to fix.

@jonas054
Copy link
Collaborator

@dblock Take a look at AccessModifierIndentation#class_constructor?. I think you should move it to the AccessModifierNode module and reuse it.

@dblock
Copy link
Contributor Author

dblock commented Dec 12, 2014

That worked, thanks @jonas054. PR #1491.

@dblock
Copy link
Contributor Author

dblock commented Dec 12, 2014

I PRed a solution to prevent future infinite loops in #1492.

dblock added a commit to dblock/rubocop that referenced this issue Dec 12, 2014
… violation inside method calls with names identical to an access modifier.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants