Skip to content

Commit

Permalink
feat(compiler): support directive selectors with attributes containin…
Browse files Browse the repository at this point in the history
…g `$`

This commit adds support for `$` in when selecting attributes.

Resolves angular#41244.

test(language-service): Add test to expose bug caused by source file change (angular#41500)

This commit adds a test to expose the bug caused by source file change in
between typecheck programs.

PR Close angular#41500
  • Loading branch information
iRealNirmal committed May 4, 2021
1 parent ea89617 commit 2c9ff71
Show file tree
Hide file tree
Showing 2 changed files with 143 additions and 7 deletions.
60 changes: 53 additions & 7 deletions packages/compiler/src/selector.ts
Expand Up @@ -13,11 +13,11 @@ const _SELECTOR_REGEXP = new RegExp(
'(([\\.\\#]?)[-\\w]+)|' + // 2: "tag"; 3: "."/"#";
// "-" should appear first in the regexp below as FF31 parses "[.-\w]" as a range
// 4: attribute; 5: attribute_string; 6: attribute_value
'(?:\\[([-.\\w*]+)(?:=([\"\']?)([^\\]\"\']*)\\5)?\\])|' + // "[name]", "[name=value]",
// "[name="value"]",
// "[name='value']"
'(\\))|' + // 7: ")"
'(\\s*,\\s*)', // 8: ","
'(?:\\[([-.\\w*\\\\$]+)(?:=([\"\']?)([^\\]\"\']*)\\5)?\\])|' + // "[name]", "[name=value]",
// "[name="value"]",
// "[name='value']"
'(\\))|' + // 7: ")"
'(\\s*,\\s*)', // 8: ","
'g');

/**
Expand Down Expand Up @@ -94,8 +94,10 @@ export class CssSelector {
}
}
const attribute = match[SelectorRegexp.ATTRIBUTE];

if (attribute) {
current.addAttribute(attribute, match[SelectorRegexp.ATTRIBUTE_VALUE]);
current.addAttribute(
current.unescapeAttribute(attribute), match[SelectorRegexp.ATTRIBUTE_VALUE]);
}
if (match[SelectorRegexp.NOT_END]) {
inNot = false;
Expand All @@ -113,6 +115,50 @@ export class CssSelector {
return results;
}

/**
* Unescape `\$` sequences from the CSS attribute selector.
*
* This is needed because `$` can have a special meaning in CSS selectors,
* but we might want to match an attribute that contains `$`.
* [MDN web link for more
* info](https://developer.mozilla.org/en-US/docs/Web/CSS/Attribute_selectors).
* @param attr the attribute to unescape.
* @returns the unescaped string.
*/
unescapeAttribute(attr: string): string {
let result = '';
let escaping = false;
for (let i = 0; i < attr.length; i++) {
const char = attr.charAt(i);
if (char === '\\') {
escaping = true;
continue;
}
if (char === '$' && !escaping) {
throw new Error(
`Error in attribute selector "${attr}". ` +
`Unescaped "$" is not supported. Please escape with "\\$".`);
}
escaping = false;
result += char;
}
return result;
}

/**
* Escape `$` sequences from the CSS attribute selector.
*
* This is needed because `$` can have a special meaning in CSS selectors,
* with this method we are escaping `$` with `\$'.
* [MDN web link for more
* info](https://developer.mozilla.org/en-US/docs/Web/CSS/Attribute_selectors).
* @param attr the attribute to escape.
* @returns the escaped string. 
*/
escapeAttribute(attr: string): string {
return attr.replace(/\\/g, '\\\\').replace(/\$/g, '\\$');
}

isElementSelector(): boolean {
return this.hasElementSelector() && this.classNames.length == 0 && this.attrs.length == 0 &&
this.notSelectors.length === 0;
Expand Down Expand Up @@ -165,7 +211,7 @@ export class CssSelector {
}
if (this.attrs) {
for (let i = 0; i < this.attrs.length; i += 2) {
const name = this.attrs[i];
const name = this.escapeAttribute(this.attrs[i]);
const value = this.attrs[i + 1];
res += `[${name}${value ? '=' + value : ''}]`;
}
Expand Down
90 changes: 90 additions & 0 deletions packages/compiler/test/selector/selector_spec.ts
Expand Up @@ -126,6 +126,67 @@ import {el} from '@angular/platform-browser/testing/src/browser_util';
expect(matched).toEqual([s1[0], 1]);
});

it('should support "$" in attribute names', () => {
matcher.addSelectables(s1 = CssSelector.parse('[someAttr\\$]'), 1);

expect(matcher.match(getSelectorFor({attrs[['someAttr', '']]}), selectableCollector))
.toEqual(false);
expect(matched).toEqual([]);
reset();

expect(matcher.match(getSelectorFor({attrs[['someAttr$', '']]}), selectableCollector))
.toEqual(true);
expect(matched).toEqual([s1[0], 1]);
reset();

matcher.addSelectables(s1 = CssSelector.parse('[some\\$attr]'), 1);

expect(matcher.match(getSelectorFor({attrs[['someattr', '']]}), selectableCollector))
.toEqual(false);
expect(matched).toEqual([]);

expect(matcher.match(getSelectorFor({attrs[['some$attr', '']]}), selectableCollector))
.toEqual(true);
expect(matched).toEqual([s1[0], 1]);
reset();

matcher.addSelectables(s1 = CssSelector.parse('[\\$someAttr]'), 1);

expect(matcher.match(getSelectorFor({attrs[['someAttr', '']]}), selectableCollector))
.toEqual(false);
expect(matched).toEqual([]);

expect(matcher.match(getSelectorFor({attrs[['$someAttr', '']]}), selectableCollector))
.toEqual(true);
expect(matched).toEqual([s1[0], 1]);
reset();

matcher.addSelectables(s1 = CssSelector.parse('[some-\\$Attr]'), 1);
matcher.addSelectables(s2 = CssSelector.parse('[some-\\$Attr][some-\\$-attr]'), 2);

expect(matcher.match(getSelectorFor({attrs[['some\\$Attr', '']]}), selectableCollector))
.toEqual(false);
expect(matched).toEqual([]);

expect(matcher.match(
getSelectorFor({attrs: [['some-$-attr', 'someValue'], ['some-$Attr', '']]}),
selectableCollector))
.toEqual(true);
expect(matched).toEqual([s1[0], 1, s2[0], 2]);
reset();


expect(matcher.match(getSelectorFor({attrs[['someattr$', '']]}), selectableCollector))
.toEqual(false);
expect(matched).toEqual([]);

expect(matcher.match(
getSelectorFor({attrs[['some-simple-attr', '']]}), selectableCollector))
.toEqual(false);
expect(matched).toEqual([]);
reset();
});

it('should select by attr name only once if the value is from the DOM', () => {
matcher.addSelectables(s1 = CssSelector.parse('[some-decor]'), 1);

Expand Down Expand Up @@ -307,6 +368,35 @@ import {el} from '@angular/platform-browser/testing/src/browser_util';
expect(cssSelector.toString()).toEqual('sometag');
});

it('should detect attr names with escaped $', () => {
let cssSelector = CssSelector.parse('[attrname\\$]')[0];
expect(cssSelector.attrs).toEqual(['attrname$', '']);
expect(cssSelector.toString()).toEqual('[attrname\\$]');

cssSelector = CssSelector.parse('[\\$attrname]')[0];
expect(cssSelector.attrs).toEqual(['$attrname', '']);
expect(cssSelector.toString()).toEqual('[\\$attrname]');

cssSelector = CssSelector.parse('[foo\\$bar]')[0];
expect(cssSelector.attrs).toEqual(['foo$bar', '']);
expect(cssSelector.toString()).toEqual('[foo\\$bar]');
});

it('should error on attr names with unescaped $', () => {
expect(() => CssSelector.parse('[attrname$]'))
.toThrowError(
'Error in attribute selector "attrname$". Unescaped "$" is not supported. Please escape with "\\$".');
expect(() => CssSelector.parse('[$attrname]'))
.toThrowError(
'Error in attribute selector "$attrname". Unescaped "$" is not supported. Please escape with "\\$".');
expect(() => CssSelector.parse('[foo$bar]'))
.toThrowError(
'Error in attribute selector "foo$bar". Unescaped "$" is not supported. Please escape with "\\$".');
expect(() => CssSelector.parse('[foo\\$bar$]'))
.toThrowError(
'Error in attribute selector "foo\\$bar$". Unescaped "$" is not supported. Please escape with "\\$".');
});

it('should detect class names', () => {
const cssSelector = CssSelector.parse('.someClass')[0];
expect(cssSelector.classNames).toEqual(['someclass']);
Expand Down

0 comments on commit 2c9ff71

Please sign in to comment.