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.

Sign up for GitHub

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

Jump to bottom

Add Lint/Unused{Local,Instance,Class}VariableAccess #552

Merged
merged 12 commits into from
Feb 17, 2025
Merged

Add Lint/Unused{Local,Instance,Class}VariableAccess #552

merged 12 commits into from
Feb 17, 2025

Conversation

nobodywasishere
Copy link
Contributor

Closes #542

@Sija Sija added the rule label Jan 29, 2025
@Sija Sija added this to the 1.7.0 milestone Jan 29, 2025
@nobodywasishere nobodywasishere marked this pull request as draft January 29, 2025 18:53
@nobodywasishere nobodywasishere force-pushed the nobody/unused-local-instance-class-variable-access branch 2 times, most recently from dd20bdc to 12cb00a Compare February 12, 2025 04:33
@nobodywasishere nobodywasishere marked this pull request as ready for review February 12, 2025 06:42
@nobodywasishere
Copy link
Contributor Author

Just need to update the specs

@Sija
Copy link
Member

Sija commented Feb 12, 2025 •
edited
Loading

I'm wondering, what's the point of node_is_used argument if all of the usages of ImplicitReturnVisitor rely on the fact of it being false?

@nobodywasishere
Copy link
Contributor Author

It helps during development to differentiate between "the visitor never reached this node" and "the visitor considers this node used"

@Sija
Copy link
Member

Sija commented Feb 12, 2025

@nobodywasishere Perhaps, yet I see no point of having this baked in.

@nobodywasishere nobodywasishere force-pushed the nobody/unused-local-instance-class-variable-access branch from 809ac1b to dba8385 Compare February 12, 2025 18:20
@nobodywasishere nobodywasishere force-pushed the nobody/unused-local-instance-class-variable-access branch from dba8385 to 258a812 Compare February 12, 2025 20:22
Sija
Sija requested changes Feb 14, 2025
View reviewed changes
.vscode/settings.json Outdated Show resolved Hide resolved
spec/ameba/rule/lint/unused_local_variable_access_spec.cr Outdated Show resolved Hide resolved
@nobodywasishere nobodywasishere force-pushed the nobody/unused-local-instance-class-variable-access branch from 1b28774 to fa7244e Compare February 14, 2025 20:39
@Sija Sija requested a review from veelenga February 14, 2025 21:31
veelenga
veelenga approved these changes Feb 17, 2025
View reviewed changes
Copy link
Member

@veelenga veelenga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@Sija Sija merged commit 70dc11d into crystal-ameba:master Feb 17, 2025
4 checks passed
@nobodywasishere nobodywasishere deleted the nobody/unused-local-instance-class-variable-access branch February 17, 2025 16:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@straight-shoota straight-shoota straight-shoota left review comments

@Sija Sija Sija approved these changes

@veelenga veelenga veelenga approved these changes

Assignees

@nobodywasishere nobodywasishere

Labels
rule
Projects
None yet
Milestone
1.7.0
Development

Successfully merging this pull request may close these issues.

Add Lint/Unused{Local,Instance,Class}VariableAccess
4 participants
@nobodywasishere @Sija @straight-shoota @veelenga