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

False Style/RedundantSelf on static method #12512

Open
abitrolly opened this issue Dec 4, 2023 · 8 comments
Open

False Style/RedundantSelf on static method #12512

abitrolly opened this issue Dec 4, 2023 · 8 comments

Comments

@abitrolly
Copy link

Actual behavior

In this GitHub job https://github.com/Homebrew/brew/actions/runs/7091523800/job/19300839843?pr=16238#step:6:15

image

self.name_for_url is the static method in the same class. Removing self from it results in the following error.

image

Expected behavior

Let static method calls pass. Or provide sufficient explanation why static method calls are bad.

Steps to reproduce the problem

This is extremely important! Providing us with a reliable way to reproduce
a problem will expedite its solution.

RuboCop version

Include the output of rubocop -V or bundle exec rubocop -V if using Bundler.
If you see extension cop versions (e.g. rubocop-performance, rubocop-rspec, and others)
output by rubocop -V, include them as well. Here's an example:

$ [bundle exec] rubocop -V
1.58.0 (using Parser 3.2.2.3, rubocop-ast 1.29.0, running on ruby 3.2.2) [x86_64-linux]
  - rubocop-performance 1.18.0
  - rubocop-rspec 2.23.2
@vlad-pisanov
Copy link

If name_from_url is a class method, then calling self.name_from_url from an instance method wouldn't work anyways. You'd have to do self.class.name_from_url.

At least that's what's happening in: https://github.com/Homebrew/brew/blob/9f65f120f3fef7263a646d3f690098751c02e078/Library/Homebrew/formula_creator.rb#L50-L51

@abitrolly
Copy link
Author

abitrolly commented Dec 5, 2023

You're right. The self.name_from_url didn't work. The syntax that works is FormulaCreator.name_from_url without self.

I still feel that rubocop should be smarter in this way and do not propose the misleading solution. The case like this should have been caught by tests, but test are often run only after linter.

@fatkodima
Copy link
Contributor

I still feel that rubocop should be smarter in this way and do not propose the misleading solution. The case like this should have been caught by tests, but test are often run only after linter.

There is no way rubocop can help with this.

@abitrolly
Copy link
Author

@fatkodima can rubocop detect that there is no self.whatever method?

@fatkodima
Copy link
Contributor

No, rubocop is a static analysis tool, meaning that it analyzes code without executing it.

@abitrolly
Copy link
Author

@fatkodima I am not an expert in Ruby static analysis. Does rubocop read a list of methods in a class?

@fatkodima
Copy link
Contributor

It can read, but it does not do that most of the time. And this will be hard (read "impossible"), because you can define methods in parent classes, inside included modules, via metaprogramming.

@abitrolly
Copy link
Author

@fatkodima but rubocop does read... no? Well, I see now. The message "Method .... does not exist" in the issue description comes from sorbet check.

I guess there is no way rubocop can query sorbet for a list of valid methods in this context.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants