Added rule avoid_final_parameters #3045
Conversation
Added rule avoid_final_parameters
|
This looks great. Could you add yourself to AUTHORS while you're at it? |
| var visitor = _Visitor(this); | ||
| registry.addConstructorDeclaration(this, visitor); | ||
| registry.addFunctionExpression(this, visitor); | ||
| registry.addMethodDeclaration(this, visitor); |
There was a problem hiding this comment.
Although there are a couple of places where FormalParameterList can appear in the AST that aren't covered here, I believe that final is invalid in those places. As a result, it might be easier today, and require less maintenance in the future, to register to visit FormalParameterList directly than to register to visit the parents.
There was a problem hiding this comment.
@bwilkerson Just to be on the same page, can you provide 1 or 2 small examples of these other places ? To stay as consistent as possible, I made this rule reflect the opposite of prefer_final_parameters. That being said, I wouldn't mind doing slight changes to this PR if I understand them lol.
There was a problem hiding this comment.
In the course of replying I decided that it's better to leave the code the way you have it. Sorry for the noise.
Nodes of type FormalParameterList appear as a child of the following node types: ConstructorDeclaration, FieldFormalParameter, FunctionExpression, FunctionTypeAlias, FunctionTypedFormalParameter, GenericFunctionType, and MethodDeclaration. The ones that are currently missing are in typedefs, both the old and new syntactic form, and in formal parameters whose type is a function. I believe that in all those locations any use of final is an error. Which means that if you did what I suggested we'd be double reporting in those places, which is something we strive to avoid.
* Added rule avoid_final_parameters Added rule avoid_final_parameters * Update AUTHORS
Added rule avoid_final_parameters
Description
This is a new lint rule to be used with
parameter_assignments. It is a style rule.Tests passed on my end (ran on Windows 10).
Fixes #3036