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

Unused instance and local variable accesses #15455

Open
nobodywasishere opened this issue Feb 12, 2025 · 4 comments
Open

Unused instance and local variable accesses #15455

nobodywasishere opened this issue Feb 12, 2025 · 4 comments

Comments

@nobodywasishere
Copy link
Contributor

I've been developing new rules for ameba, and as part of that have been testing them on this repository to verify their functionality. I've found a few issues that could be fixed.

src/compiler/crystal/tools/formatter.cr:

Image

src/compiler/crystal/syntax/parser.cr:

Image

src/compiler/crystal/semantic/filters.cr:

Image

There are several others, but they're mostly having the var on the last line of a method that has a Nil return.

@nobodywasishere nobodywasishere added the kind:bug A bug in the code. Does not apply to documentation, specs, etc. label Feb 12, 2025
@Blacksmoke16 Blacksmoke16 added kind:chore and removed kind:bug A bug in the code. Does not apply to documentation, specs, etc. labels Feb 12, 2025
@straight-shoota
Copy link
Member

Happy to accept a patch.

I'm not sure it makes much sense to track such individual linter issues in a ticket.
As long as the code works, it's fine. Of course we'd want to improve the quality.
But IMO it's not important enough to keep a bookmark for every detail. Especially considering that they should be relatively straightforward to just fix.
We could have a meta issue to keep track of linter problems, though.

@nobodywasishere
Copy link
Contributor Author

I agree, I'll start working on an initial PR to start fixing these and other issues. The check_end one concerns me though, as it may introduce a breaking change by fixing it. I'm fine turning this into a ticket to keep track of linter issues / discuss them

@straight-shoota
Copy link
Member

Related:

@straight-shoota
Copy link
Member

Of course, if there's a problematic detail to discuss on a specific linter issue, such a discussion is much appreciated.

It wasn't clear to me that the check_end variable is shadowing a method. So yeah we ought to be careful about that. Apparently the formatter specs are not dense enough to catch that these check_end are not actually checking the end.

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