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

Add Lint/UselessDef and Lint/UselessVisibilityModifier #559

Open
nobodywasishere opened this issue Jan 31, 2025 · 4 comments
Open

Add Lint/UselessDef and Lint/UselessVisibilityModifier #559

nobodywasishere opened this issue Jan 31, 2025 · 4 comments
Assignees
Milestone

Comments

@nobodywasishere
Copy link
Contributor

This may be something that should be a compiler warning instead, so feel free to close if it should be. It's possible to define operator methods top-level that are impossible to call:

def +(other)
  p! other
end

+1 # => 1

As well, it's possible to define a method with protected top-level, when the visibility modifier has no effect

protected def hello(a)
  p! a
end

hello(1)
# a # => 1

See: crystal-lang/crystal#12360

@straight-shoota
Copy link
Contributor

Since these methods cannot be called and the visibility restriction makes no sense, I suppose this could find its place in the compiler. Feel free to start adopting it in ameba though.

@nobodywasishere
Copy link
Contributor Author

My personal preference for these (and other things that would give compiler warnings) is to have them in both, due to ameba's integration with dev environments being better + a better system for reporting issues.

@nobodywasishere nobodywasishere changed the title Add Lint/TopLevelOperatorMethod and Lint/UselessVisibilityModifier Add Lint/UselessDef and Lint/UselessVisibilityModifier Jan 31, 2025
@Sija
Copy link
Member

Sija commented Feb 2, 2025

I also think it's a good idea to have these rules.

@nobodywasishere
Copy link
Contributor Author

I'll take a look at implementing these once we get through the current set of PRs.

@Sija Sija added this to the 1.7.0 milestone Feb 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants