-
Notifications
You must be signed in to change notification settings - Fork 284
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
Memoization of rule arguments #595
Conversation
rule/cyclomatic.go
Outdated
|
||
// Apply applies the rule to given file. | ||
func (r *CyclomaticRule) Apply(file *lint.File, arguments lint.Arguments) []lint.Failure { | ||
checkNumberOfArguments(1, arguments, r.Name()) | ||
if r.maxComplexity < 1 { |
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.
Maybe == 0
for consistency?
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.
done
rule/argument-limit.go
Outdated
|
||
// Apply applies the rule to given file. | ||
func (r *ArgumentsLimitRule) Apply(file *lint.File, arguments lint.Arguments) []lint.Failure { | ||
checkNumberOfArguments(1, arguments, r.Name()) | ||
if r.total < 1 { |
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.
Maybe == 0
for consistency?
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.
done
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.
LGTM. Left minor comments. Both > 1
and == 0
sound good. To me == 0
is a bit better, more deterministic.
Co-authored-by: doniacld <[email protected]>
In the current implementation of rules, the configuration of rules is read and checked each time the rule is applied to a file.
For example in
revive/rule/banned-characters.go
Lines 17 to 33 in b331445
lines 20 and 21 will be executed for each file with the very same
arguments
argument, thus the function calls will return always the same result.This PR modifies the way argument of rules are handled by memoizing them and avoid checking and analyzing them for each file.