-
Notifications
You must be signed in to change notification settings - Fork 38
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/UnusedTypeOrConstant
#537
base: master
Are you sure you want to change the base?
Add Lint/UnusedTypeOrConstant
#537
Conversation
incr_stack do | ||
node.obj.try &.accept(self) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Needed to change this as otherwise a top-level MyClass.run
would be flagged. It does mean things like "hello".to_s
will no longer be flagged as unused
|
Lint/UnusedPathGenericOrUnion
Lint/UnusedTypeOrConstant
def hello | ||
Float64 | ||
# ^^^^^^^ error: Path or generic type is not used | ||
0.1 | ||
end | ||
|
||
fun fun_name : Int32 | ||
Int32 | ||
# ^^^^^ error: Path or generic type is not used | ||
1234 | ||
end | ||
|
||
class MyClass | ||
MyModule | ||
# ^^^^^^^^ error: Path or generic type is not used | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these are not "top level" as mentioned in the spec name. Can we move it to a separate testcase?
it "passes if paths and generic types are used" do | ||
expect_no_issues subject, <<-CRYSTAL | ||
MyConst = 1 | ||
|
||
my_var : String? = EMPTY_STRING | ||
|
||
my_var.as(String) | ||
|
||
puts StaticArray(Int32, 10) | ||
|
||
class MyClass < MySuperClass | ||
include MyModule | ||
extend MyModule | ||
end | ||
|
||
a : Int32 = 10 | ||
|
||
klass = String? | ||
|
||
alias MyType = Float64 | StaticArray(Float64, 10) | ||
|
||
def size : Float64 | ||
0.1 | ||
end | ||
|
||
lib MyLib | ||
type MyType = Void* | ||
|
||
struct MyStruct | ||
field1, field2 : Float64 | ||
end | ||
end | ||
|
||
fun fun_name : Int32 | ||
1234 | ||
end | ||
|
||
Int32 | "Float64" | ||
|
||
MyClass.run | ||
CRYSTAL | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i would suggest to slipt this into smaller testcases as well. Otherwise, new examples will just be added to the same big testcase in the future.
def visit(node : Crystal::Alias) | ||
@rule.test(@source, node, @stack > 0) | ||
|
||
incr_stack do | ||
node.name.accept(self) | ||
node.value.accept(self) | ||
end | ||
|
||
false | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you recall how this visitor functions, particularly why we increment a stack and the meaning of last_is_used
in the rules? It might be helpful to document this visitor for better clarity, as it's currently challenging to understand it quickly.
Similar to
Lint/UnusedLiteral
andLint/UnusedComparison
. Not too fond of the name so any suggestions are welcome.