fix(prefer-native-locators): stop the autofix from emitting invalid code - #502
Open
dylanpulver wants to merge 1 commit into
Open
fix(prefer-native-locators): stop the autofix from emitting invalid code#502dylanpulver wants to merge 1 commit into
dylanpulver wants to merge 1 commit into
Conversation
The selector was matched with a lazy regex anchored at the end of the string, so a compound selector such as [role="dialog"][data-state="open"] matched with everything between the first quote and the last bracket captured as the attribute value. The fix then wrote that text straight into a double quoted literal, producing source that no longer parses. The emitted literal was also always double quoted and never escaped, so a value containing a double quote broke the same way. The selector is now matched only when it is exactly one attribute selector, with the value either quoted or a bare run of characters. Anything carrying a descendant, a combinator or a second attribute no longer matches, since no single native locator selects the same elements. The value is unescaped and then rendered with a quote style that suits its contents.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Running
eslint --fixwith this rule enabled can leave source files that nolonger parse. On a compound selector the fixer writes the rest of the selector
into the middle of the string literal it emits, so the file is corrupted rather
than fixed. In a pre-commit hook or a CI autofix step that lands silently. The
reporter in #482 had four files broken by one run.
Two selector shapes trigger it.
A selector that starts with a matched attribute and then continues:
becomes
A value that contains a double quote, since the emitted literal is always double
quoted and the value is never escaped:
becomes
I reproduced both on
mainbefore touching anything.Cause
compilePatternsbuilds^\[<attr>=['"]?(.+?)['"]?\]$for each attribute. Thecapture group is lazy, but the pattern is anchored at the end of the string, so
on a compound selector the group has to expand past the closing bracket of the
first attribute and swallow everything up to the last one. That captured text is
then interpolated into
"${match[1]}"with no escaping applied.Change
A selector is now matched only when it is exactly one attribute selector for the
attribute in question. The value is either quoted, where the escapes permitted
inside it are for a quote or for a backslash, or a bare run of characters with no
whitespace, quotes, brackets or backslashes in it.
A selector that also carries a descendant or a second attribute no longer
matches, so nothing is reported for it and no fix is offered. That lines up with
how the rule already treats a selector with a leading class or tag, and it is the
right outcome because no single native locator selects the same elements. Both
getByTestId('a')andgetByRole('dialog')would have quietly dropped aconstraint even if the syntax had come out right.
The captured value is unescaped and then rendered with a quote style picked from
its contents. Double quotes remain the default so existing output is unchanged. A
value holding a double quote and no single quote is emitted in single quotes, and
a value holding both is emitted in double quotes with the inner ones escaped.
One small related thing: the attribute name is escaped before it goes into the
regex, since
testIdAttributecomes from user config.Tests
src/rules/prefer-native-locators.test.tsgains valid cases for the compoundshapes from the report, covering the ones whose autofix output previously did not
parse, plus invalid cases for a value containing a double quote and for a value
containing both quote characters. Nine of the added cases fail against the
current rule and pass with this change. Every existing case still passes
untouched, and
yarn ciis green.Fixes #482