-
Notifications
You must be signed in to change notification settings - Fork 5
NEW @W-19772057@ Enhanced rule selector support #363
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
Conversation
204e13f
to
d188bb3
Compare
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.
see my teeny edits!
`No rule with name '%s' and engine '%s' exists among the selected rules.`, | ||
|
||
SelectorCannotBeEmpty: | ||
`Rule selectors cannot be empty strings.`, |
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.
`Rule selectors cannot be empty strings.`, | |
`Rule selectors can't be empty strings.`, |
`Rule selectors cannot be empty strings.`, | ||
|
||
SelectorLooksIncorrect: | ||
`Rule selector '%s' looks incorrect. Make sure that parentheses are balanced, and that all subselectors are joined by a colon (:) or comma (,).`, |
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.
`Rule selector '%s' looks incorrect. Make sure that parentheses are balanced, and that all subselectors are joined by a colon (:) or comma (,).`, | |
`Rule selector '%s' looks incorrect. Make sure that parentheses are balanced and that all subselectors are joined by a colon (:) or comma (,).`, |
`Rule selector '%s' looks incorrect. Make sure that parentheses are balanced, and that all subselectors are joined by a colon (:) or comma (,).`, | ||
|
||
EngineRunResultsMissing: | ||
`Could to get results for engine '%s' since they are missing from the overall run results. Most likely the engine did not run.`, |
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 to get results for engine '%s' since they are missing from the overall run results. Most likely the engine did not run.`, | |
`Couldn't get results for engine '%s' since they're missing from the overall run results. Most likely the engine didn't run.`, |
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.
Nice spot @jshackell-sfdc!
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.
A few clarifying questions
stubEngine2Rules: ['stub2RuleC'] | ||
}, | ||
{ | ||
selector: 'Recommended,3:Performance', // Equivalent to "(Recommended,4):Performance", or "(Recommended:Performance),(4:Performance)" |
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.
selector: 'Recommended,3:Performance', // Equivalent to "(Recommended,4):Performance", or "(Recommended:Performance),(4:Performance)" | |
selector: 'Recommended,3:Performance', // Equivalent to "(Recommended,3):Performance", or "(Recommended:Performance),(3:Performance)" |
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.
Oh, good catch. Thanks.
case: 'colons and commas are nested via parentheses', | ||
selectors: ['(Recommended:Performance),(stubEngine2:2),(stubEngine2:DoesNotExist)'] | ||
} | ||
])('When $case, then we get correct union and intersection behavior', async ({selectors}) => { |
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.
Is there a logical limit to how many unions and intersections there can be, or does the limit not exist?
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.
The algorithm is just standard recursion, so there's no hardcoded limit.
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.
This is a nice enhancement! Let's plan to add more doc changes in the CLI as well to help our developers understand how to use these new selectors.
Previously, rule selectors could use colons (
:
) to join multiple criteria with a logical AND (e.g.,Performance:2
is all Performance rules with severity 2).After this change, they can also use commas (
,
) to join criteria with a logical OR (Performance,2
would be all Performance rules and all rules with severity 2). Furthermore, parentheses can be used to create groups that are handled effectively as logical booleans.E.g.,
(Performance:2),(eslint:1)
would be all performance rules with sev2, and every ESLint Sev1 rule.