From c91d5bf1cedfe3957001d148b6055835cf66d8f4 Mon Sep 17 00:00:00 2001 From: Pine Wu Date: Thu, 14 Jun 2018 10:38:35 -0700 Subject: [PATCH 1/5] Handle unknown at rules. Fix #51 --- src/parser/cssErrors.ts | 1 + src/parser/cssNodes.ts | 14 ++++++++++- src/parser/cssParser.ts | 45 ++++++++++++++++++++++++++++-------- src/parser/cssScanner.ts | 16 ++++++------- src/parser/lessParser.ts | 10 +++++--- src/parser/scssParser.ts | 9 +++----- src/test/css/parser.test.ts | 15 ++++++++++-- src/test/less/parser.test.ts | 3 +-- src/test/scss/parser.test.ts | 2 +- 9 files changed, 83 insertions(+), 32 deletions(-) diff --git a/src/parser/cssErrors.ts b/src/parser/cssErrors.ts index ef505da6..37cc2211 100644 --- a/src/parser/cssErrors.ts +++ b/src/parser/cssErrors.ts @@ -45,6 +45,7 @@ export let ParseError = { CommaExpected: new CSSIssueType('css-commaexpected', localize('expected.comma', "comma expected")), PageDirectiveOrDeclarationExpected: new CSSIssueType('css-pagedirordeclexpected', localize('expected.pagedirordecl', "page directive or declaraton expected")), UnknownAtRule: new CSSIssueType('css-unknownatrule', localize('unknown.atrule', "at-rule unknown")), + AtRuleBodyExpected: new CSSIssueType('css-atrulebodyexpected', localize('expected.atrulebody', 'at rule body expected')), UnknownKeyword: new CSSIssueType('css-unknownkeyword', localize('unknown.keyword', "unknown keyword")), SelectorExpected: new CSSIssueType('css-selectorexpected', localize('expected.selector', "selector expected")), StringLiteralExpected: new CSSIssueType('css-stringliteralexpected', localize('expected.stringliteral', "string literal expected")), diff --git a/src/parser/cssNodes.ts b/src/parser/cssNodes.ts index 77eda9e9..cf5c7058 100644 --- a/src/parser/cssNodes.ts +++ b/src/parser/cssNodes.ts @@ -81,7 +81,8 @@ export enum NodeType { SupportsCondition, NamespacePrefix, GridLine, - Plugin + Plugin, + UnknownAtRule, } export enum ReferenceType { @@ -1486,6 +1487,17 @@ export class MixinDeclaration extends BodyDeclaration { } } +export class UnknownAtRule extends BodyDeclaration { + + constructor(offset: number, length: number) { + super(offset, length); + } + + public get type(): NodeType { + return NodeType.UnknownAtRule; + } +} + export class ListEntry extends Node { public key?: Node; diff --git a/src/parser/cssParser.ts b/src/parser/cssParser.ts index 3fce1811..eac112ea 100644 --- a/src/parser/cssParser.ts +++ b/src/parser/cssParser.ts @@ -272,19 +272,24 @@ export class Parser { public _parseStylesheetStatement(): nodes.Node { if (this.peek(TokenType.AtKeyword)) { - return this._parseImport() - || this._parseMedia() - || this._parsePage() - || this._parseFontFace() - || this._parseKeyframe() - || this._parseSupports() - || this._parseViewPort() - || this._parseNamespace() - || this._parseDocument(); + return this._parseStylesheetAtStatement(); } return this._parseRuleset(false); } + public _parseStylesheetAtStatement(): nodes.Node { + return this._parseImport() + || this._parseMedia() + || this._parsePage() + || this._parseFontFace() + || this._parseKeyframe() + || this._parseSupports() + || this._parseViewPort() + || this._parseNamespace() + || this._parseDocument() + || this._parseUnknownAtRule(); + } + public _tryParseRuleset(isNested: boolean): nodes.RuleSet { let mark = this.mark(); if (this._parseSelector(isNested)) { @@ -992,6 +997,28 @@ export class Parser { return this._parseBody(node, this._parseStylesheetStatement.bind(this)); } + // https://www.w3.org/TR/css-syntax-3/#consume-an-at-rule + public _parseUnknownAtRule(): nodes.Node { + if (!this.peek(TokenType.AtKeyword)) { + return null; + } + + let node = this.create(nodes.UnknownAtRule); + this.consumeToken(); + + this.resync([], [TokenType.SemiColon, TokenType.EOF, TokenType.CurlyL]); // ignore all the rules + if (this.peek(TokenType.SemiColon)) { + this.consumeToken(); + return node; + } else if (this.peek(TokenType.CurlyL)) { + return this._parseBody(node, this._parseStylesheetStatement.bind(this)); + } else if (this.peek(TokenType.EOF)) { + return this.finish(node, ParseError.AtRuleBodyExpected); + } + + return node; + } + public _parseOperator(): nodes.Operator { // these are operators for binary expressions if (this.peekDelim('/') || diff --git a/src/parser/cssScanner.ts b/src/parser/cssScanner.ts index bf7538f7..077f638c 100644 --- a/src/parser/cssScanner.ts +++ b/src/parser/cssScanner.ts @@ -15,8 +15,8 @@ export enum TokenType { Percentage, Dimension, UnicodeRange, - CDO, - CDC, + CDO, // Colon, SemiColon, CurlyL, @@ -27,13 +27,13 @@ export enum TokenType { BracketR, Whitespace, Includes, - Dashmatch, - SubstringOperator, - PrefixOperator, - SuffixOperator, + Dashmatch, // |= + SubstringOperator, // *= + PrefixOperator, // ^= + SuffixOperator, // $= Delim, - EMS, - EXS, + EMS, // 3em + EXS, // 3ex Length, Angle, Time, diff --git a/src/parser/lessParser.ts b/src/parser/lessParser.ts index d0faba9a..3a1d3bae 100644 --- a/src/parser/lessParser.ts +++ b/src/parser/lessParser.ts @@ -21,11 +21,15 @@ export class LESSParser extends cssParser.Parser { } public _parseStylesheetStatement(): nodes.Node { + if (this.peek(TokenType.AtKeyword)) { + return this._parseVariableDeclaration() + || this._parsePlugin() + || super._parseStylesheetAtStatement(); + } + return this._tryParseMixinDeclaration() || this._tryParseMixinReference(true) - || super._parseStylesheetStatement() - || this._parseVariableDeclaration() - || this._parsePlugin(); + || this._parseRuleset(true); } public _parseImport(): nodes.Node { diff --git a/src/parser/scssParser.ts b/src/parser/scssParser.ts index 4311e9f0..7f2fe34c 100644 --- a/src/parser/scssParser.ts +++ b/src/parser/scssParser.ts @@ -22,19 +22,16 @@ export class SCSSParser extends cssParser.Parser { } public _parseStylesheetStatement(): nodes.Node { - let node = super._parseStylesheetStatement(); - if (node) { - return node; - } if (this.peek(TokenType.AtKeyword)) { return this._parseWarnAndDebug() || this._parseControlStatement() || this._parseMixinDeclaration() || this._parseMixinContent() || this._parseMixinReference() // @include - || this._parseFunctionDeclaration(); + || this._parseFunctionDeclaration() + || super._parseStylesheetAtStatement(); } - return this._parseVariableDeclaration(); + return this._parseRuleset(true) || this._parseVariableDeclaration(); } public _parseImport(): nodes.Node { diff --git a/src/test/css/parser.test.ts b/src/test/css/parser.test.ts index 7c38acd5..b790177d 100644 --- a/src/test/css/parser.test.ts +++ b/src/test/css/parser.test.ts @@ -71,11 +71,22 @@ suite('CSS - Parser', () => { assertNode('E.warning E#myid E:not(s) {}', parser, parser._parseStylesheet.bind(parser)); assertError('@namespace;', parser, parser._parseStylesheet.bind(parser), ParseError.URIExpected); assertError('@namespace url(http://test)', parser, parser._parseStylesheet.bind(parser), ParseError.SemiColonExpected); - assertError('@mskeyframes darkWordHighlight { from { background-color: inherit; } to { background-color: rgba(83, 83, 83, 0.7); } }', parser, parser._parseStylesheet.bind(parser), ParseError.UnknownAtRule); assertError('@charset;', parser, parser._parseStylesheet.bind(parser), ParseError.IdentifierExpected); assertError('@charset \'utf8\'', parser, parser._parseStylesheet.bind(parser), ParseError.SemiColonExpected); }); + test('stylesheet - graceful handling of unknown rules', function () { + let parser = new Parser(); + assertNode('@unknown-rule;', parser, parser._parseStylesheet.bind(parser)); + assertNode('@unknown-rule (;', parser, parser._parseStylesheet.bind(parser)); + assertNode(`@unknown-rule 'foo';`, parser, parser._parseStylesheet.bind(parser)); + assertNode('@unknown-rule (foo) {}', parser, parser._parseStylesheet.bind(parser)); + assertNode('@unknown-rule (foo) { .bar {} }', parser, parser._parseStylesheet.bind(parser)); + assertNode('@mskeyframes darkWordHighlight { from { background-color: inherit; } to { background-color: rgba(83, 83, 83, 0.7); } }', parser, parser._parseStylesheet.bind(parser)); + + assertError('@unknown-rule (foo) { .bar {}', parser, parser._parseStylesheet.bind(parser), ParseError.RightCurlyExpected); + }); + test('stylesheet /panic/', function () { let parser = new Parser(); assertError('#boo, far } \n.far boo {}', parser, parser._parseStylesheet.bind(parser), ParseError.LeftCurlyExpected); @@ -195,7 +206,7 @@ suite('CSS - Parser', () => { assertNode('@page { @top-right-corner { content: url(foo.png); border: solid green; } }', parser, parser._parsePage.bind(parser)); assertNode('@page { @top-left-corner { content: " "; border: solid green; } @bottom-right-corner { content: counter(page); border: solid green; } }', parser, parser._parsePage.bind(parser)); assertError('@page { @top-left-corner foo { content: " "; border: solid green; } }', parser, parser._parsePage.bind(parser), ParseError.LeftCurlyExpected); - assertError('@page { @XY foo { content: " "; border: solid green; } }', parser, parser._parsePage.bind(parser), ParseError.UnknownAtRule); + // assertError('@page { @XY foo { content: " "; border: solid green; } }', parser, parser._parsePage.bind(parser), ParseError.UnknownAtRule); assertError('@page :left { margin-left: 4cm margin-right: 3cm; }', parser, parser._parsePage.bind(parser), ParseError.SemiColonExpected); assertError('@page : { }', parser, parser._parsePage.bind(parser), ParseError.IdentifierExpected); assertError('@page :left, { }', parser, parser._parsePage.bind(parser), ParseError.IdentifierExpected); diff --git a/src/test/less/parser.test.ts b/src/test/less/parser.test.ts index 38215943..93be99b0 100644 --- a/src/test/less/parser.test.ts +++ b/src/test/less/parser.test.ts @@ -35,8 +35,7 @@ suite('LESS - Parser', () => { assertNode('.something { @media (max-width: 760px) { > div { display: block; } } }', parser, parser._parseStylesheet.bind(parser)); assertNode('@media (@var) {}', parser, parser._parseMedia.bind(parser)); assertNode('@media screen and (@var) {}', parser, parser._parseMedia.bind(parser)); - - assertError('@media (max-width: 760px) { + div { display: block; } }', parser, parser._parseStylesheet.bind(parser), ParseError.RightCurlyExpected); + assertNode('@media (max-width: 760px) { + div { display: block; } }', parser, parser._parseStylesheet.bind(parser)); }); test('VariableDeclaration', function () { diff --git a/src/test/scss/parser.test.ts b/src/test/scss/parser.test.ts index 08d77bae..f2724238 100644 --- a/src/test/scss/parser.test.ts +++ b/src/test/scss/parser.test.ts @@ -188,7 +188,7 @@ suite('SCSS - Parser', () => { assertNode('.something { @media (max-width: 760px) { > .test { color: blue; } } }', parser, parser._parseStylesheet.bind(parser)); assertNode('.something { @media (max-width: 760px) { ~ div { display: block; } } }', parser, parser._parseStylesheet.bind(parser)); assertNode('.something { @media (max-width: 760px) { + div { display: block; } } }', parser, parser._parseStylesheet.bind(parser)); - assertError('@media (max-width: 760px) { + div { display: block; } }', parser, parser._parseStylesheet.bind(parser), ParseError.RightCurlyExpected); + assertNode('@media (max-width: 760px) { + div { display: block; } }', parser, parser._parseStylesheet.bind(parser)); }); test('@keyframe', function () { From 90431c5673f5702e386a77ffd22aabb584a60b33 Mon Sep 17 00:00:00 2001 From: Pine Wu Date: Thu, 14 Jun 2018 10:41:11 -0700 Subject: [PATCH 2/5] Tests for unknown at rule body --- src/test/css/parser.test.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/test/css/parser.test.ts b/src/test/css/parser.test.ts index b790177d..9c3e6101 100644 --- a/src/test/css/parser.test.ts +++ b/src/test/css/parser.test.ts @@ -84,6 +84,8 @@ suite('CSS - Parser', () => { assertNode('@unknown-rule (foo) { .bar {} }', parser, parser._parseStylesheet.bind(parser)); assertNode('@mskeyframes darkWordHighlight { from { background-color: inherit; } to { background-color: rgba(83, 83, 83, 0.7); } }', parser, parser._parseStylesheet.bind(parser)); + assertError('@unknown-rule', parser, parser._parseStylesheet.bind(parser), ParseError.AtRuleBodyExpected); + assertError('@unknown-rule foo', parser, parser._parseStylesheet.bind(parser), ParseError.AtRuleBodyExpected); assertError('@unknown-rule (foo) { .bar {}', parser, parser._parseStylesheet.bind(parser), ParseError.RightCurlyExpected); }); From 34f85a2fc593b6902fa044c49c6b9280e5d76dc6 Mon Sep 17 00:00:00 2001 From: Pine Wu Date: Mon, 18 Jun 2018 11:31:55 -0700 Subject: [PATCH 3/5] Add linting option --- src/parser/cssNodes.ts | 10 +++++++++- src/services/lint.ts | 12 ++++++++++++ src/services/lintRules.ts | 1 + 3 files changed, 22 insertions(+), 1 deletion(-) diff --git a/src/parser/cssNodes.ts b/src/parser/cssNodes.ts index cf5c7058..09a96f70 100644 --- a/src/parser/cssNodes.ts +++ b/src/parser/cssNodes.ts @@ -1243,7 +1243,7 @@ export class AttributeSelector extends Node { public getValue(): BinaryExpression { return this.value; - } + } } export class Operator extends Node { @@ -1488,6 +1488,7 @@ export class MixinDeclaration extends BodyDeclaration { } export class UnknownAtRule extends BodyDeclaration { + public atRuleName: string; constructor(offset: number, length: number) { super(offset, length); @@ -1496,6 +1497,13 @@ export class UnknownAtRule extends BodyDeclaration { public get type(): NodeType { return NodeType.UnknownAtRule; } + + public setAtRuleName(atRuleName: string) { + this.atRuleName = atRuleName; + } + public getAtRuleName(atRuleName: string) { + return this.atRuleName; + } } export class ListEntry extends Node { diff --git a/src/services/lint.ts b/src/services/lint.ts index 42c55a42..90cf393c 100644 --- a/src/services/lint.ts +++ b/src/services/lint.ts @@ -136,6 +136,8 @@ export class LintVisitor implements nodes.IVisitor { public visitNode(node: nodes.Node): boolean { switch (node.type) { + case nodes.NodeType.UnknownAtRule: + return this.visitUnknownAtRule(node); case nodes.NodeType.Keyframe: return this.visitKeyframe(node); case nodes.NodeType.FontFace: @@ -162,6 +164,16 @@ export class LintVisitor implements nodes.IVisitor { this.validateKeyframes(); } + private visitUnknownAtRule(node: nodes.UnknownAtRule): boolean { + const atRuleName = node.getChild(0); + if (!atRuleName) { + return false; + } + + this.addEntry(atRuleName, Rules.UnknownAtRules, `Unknown at rule ${atRuleName.getText()}`); + return true; + } + private visitKeyframe(node: nodes.Keyframe): boolean { let keyword = node.getKeyword(); let text = keyword.getText(); diff --git a/src/services/lintRules.ts b/src/services/lintRules.ts index d952f6bf..c9439949 100644 --- a/src/services/lintRules.ts +++ b/src/services/lintRules.ts @@ -35,6 +35,7 @@ export let Rules = { HexColorLength: new Rule('hexColorLength', localize('rule.hexColor', "Hex colors must consist of three, four, six or eight hex numbers"), Error), ArgsInColorFunction: new Rule('argumentsInColorFunction', localize('rule.colorFunction', "Invalid number of parameters"), Error), UnknownProperty: new Rule('unknownProperties', localize('rule.unknownProperty', "Unknown property."), Warning), + UnknownAtRules: new Rule('unknownAtRules', localize('rule.unknownAtRules', "Unknown at-rule."), Warning), IEStarHack: new Rule('ieHack', localize('rule.ieHack', "IE hacks are only necessary when supporting IE7 and older"), Ignore), UnknownVendorSpecificProperty: new Rule('unknownVendorSpecificProperties', localize('rule.unknownVendorSpecificProperty', "Unknown vendor specific property."), Ignore), PropertyIgnoredDueToDisplay: new Rule('propertyIgnoredDueToDisplay', localize('rule.propertyIgnoredDueToDisplay', "Property is ignored due to the display."), Warning), From c81075ff8427d1c4856491b12ce0f93c2a002d68 Mon Sep 17 00:00:00 2001 From: Pine Wu Date: Mon, 18 Jun 2018 11:32:20 -0700 Subject: [PATCH 4/5] Parse according to simple block spec --- src/parser/cssParser.ts | 74 ++++++++++++++++++++++++++++++++++++++--- 1 file changed, 69 insertions(+), 5 deletions(-) diff --git a/src/parser/cssParser.ts b/src/parser/cssParser.ts index eac112ea..5bf6e082 100644 --- a/src/parser/cssParser.ts +++ b/src/parser/cssParser.ts @@ -999,12 +999,66 @@ export class Parser { // https://www.w3.org/TR/css-syntax-3/#consume-an-at-rule public _parseUnknownAtRule(): nodes.Node { - if (!this.peek(TokenType.AtKeyword)) { - return null; - } - let node = this.create(nodes.UnknownAtRule); - this.consumeToken(); + node.addChild(this._parseUnknownAtRuleName()); + + const isTopLevel = () => curlyDepth === 0 && parensDepth === 0 && bracketsDepth === 0; + let curlyDepth = 0; + let parensDepth = 0; + let bracketsDepth = 0; + done: while (true) { + switch (this.token.type) { + case TokenType.SemiColon: + if (isTopLevel()) { + break done; + } + break; + case TokenType.EOF: + if (curlyDepth > 0) { + return this.finish(node, ParseError.RightCurlyExpected); + } else if (bracketsDepth > 0) { + return this.finish(node, ParseError.RightSquareBracketExpected); + } else if (parensDepth > 0) { + return this.finish(node, ParseError.RightParenthesisExpected); + } else { + return this.finish(node); + } + case TokenType.CurlyL: + curlyDepth++; + break; + case TokenType.CurlyR: + curlyDepth--; + if (curlyDepth < 0) { + // The property value has been terminated without a semicolon, and + // this is the last declaration in the ruleset. + if (parensDepth === 0 && bracketsDepth === 0) { + break done; + } + return this.finish(node, ParseError.LeftCurlyExpected); + } + break; + case TokenType.ParenthesisL: + parensDepth++; + break; + case TokenType.ParenthesisR: + parensDepth--; + if (parensDepth < 0) { + return this.finish(node, ParseError.LeftParenthesisExpected); + } + break; + case TokenType.BracketL: + bracketsDepth++; + break; + case TokenType.BracketR: + bracketsDepth--; + if (bracketsDepth < 0) { + return this.finish(node, ParseError.LeftSquareBracketExpected); + } + break; + } + + this.consumeToken(); + } this.resync([], [TokenType.SemiColon, TokenType.EOF, TokenType.CurlyL]); // ignore all the rules if (this.peek(TokenType.SemiColon)) { @@ -1019,6 +1073,16 @@ export class Parser { return node; } + public _parseUnknownAtRuleName(): nodes.Node { + let node = this.create(nodes.Node); + + if (this.accept(TokenType.AtKeyword)) { + return this.finish(node); + } + + return node; + } + public _parseOperator(): nodes.Operator { // these are operators for binary expressions if (this.peekDelim('/') || From e284752557fe0fbfa16b6d417ebd1fdb24d4e33b Mon Sep 17 00:00:00 2001 From: Pine Wu Date: Mon, 18 Jun 2018 11:40:10 -0700 Subject: [PATCH 5/5] Remove atRuleBody error and follow simple block spec for errors --- src/parser/cssErrors.ts | 1 - src/parser/cssParser.ts | 10 ---------- src/test/css/parser.test.ts | 7 ++++--- 3 files changed, 4 insertions(+), 14 deletions(-) diff --git a/src/parser/cssErrors.ts b/src/parser/cssErrors.ts index 37cc2211..ef505da6 100644 --- a/src/parser/cssErrors.ts +++ b/src/parser/cssErrors.ts @@ -45,7 +45,6 @@ export let ParseError = { CommaExpected: new CSSIssueType('css-commaexpected', localize('expected.comma', "comma expected")), PageDirectiveOrDeclarationExpected: new CSSIssueType('css-pagedirordeclexpected', localize('expected.pagedirordecl', "page directive or declaraton expected")), UnknownAtRule: new CSSIssueType('css-unknownatrule', localize('unknown.atrule', "at-rule unknown")), - AtRuleBodyExpected: new CSSIssueType('css-atrulebodyexpected', localize('expected.atrulebody', 'at rule body expected')), UnknownKeyword: new CSSIssueType('css-unknownkeyword', localize('unknown.keyword', "unknown keyword")), SelectorExpected: new CSSIssueType('css-selectorexpected', localize('expected.selector', "selector expected")), StringLiteralExpected: new CSSIssueType('css-stringliteralexpected', localize('expected.stringliteral', "string literal expected")), diff --git a/src/parser/cssParser.ts b/src/parser/cssParser.ts index 5bf6e082..e0de3bf8 100644 --- a/src/parser/cssParser.ts +++ b/src/parser/cssParser.ts @@ -1060,16 +1060,6 @@ export class Parser { this.consumeToken(); } - this.resync([], [TokenType.SemiColon, TokenType.EOF, TokenType.CurlyL]); // ignore all the rules - if (this.peek(TokenType.SemiColon)) { - this.consumeToken(); - return node; - } else if (this.peek(TokenType.CurlyL)) { - return this._parseBody(node, this._parseStylesheetStatement.bind(this)); - } else if (this.peek(TokenType.EOF)) { - return this.finish(node, ParseError.AtRuleBodyExpected); - } - return node; } diff --git a/src/test/css/parser.test.ts b/src/test/css/parser.test.ts index 9c3e6101..c24016a8 100644 --- a/src/test/css/parser.test.ts +++ b/src/test/css/parser.test.ts @@ -78,14 +78,15 @@ suite('CSS - Parser', () => { test('stylesheet - graceful handling of unknown rules', function () { let parser = new Parser(); assertNode('@unknown-rule;', parser, parser._parseStylesheet.bind(parser)); - assertNode('@unknown-rule (;', parser, parser._parseStylesheet.bind(parser)); assertNode(`@unknown-rule 'foo';`, parser, parser._parseStylesheet.bind(parser)); assertNode('@unknown-rule (foo) {}', parser, parser._parseStylesheet.bind(parser)); assertNode('@unknown-rule (foo) { .bar {} }', parser, parser._parseStylesheet.bind(parser)); assertNode('@mskeyframes darkWordHighlight { from { background-color: inherit; } to { background-color: rgba(83, 83, 83, 0.7); } }', parser, parser._parseStylesheet.bind(parser)); - assertError('@unknown-rule', parser, parser._parseStylesheet.bind(parser), ParseError.AtRuleBodyExpected); - assertError('@unknown-rule foo', parser, parser._parseStylesheet.bind(parser), ParseError.AtRuleBodyExpected); + assertError('@unknown-rule (;', parser, parser._parseStylesheet.bind(parser), ParseError.RightParenthesisExpected); + assertError('@unknown-rule [foo', parser, parser._parseStylesheet.bind(parser), ParseError.RightSquareBracketExpected); + assertError('@unknown-rule { [foo }', parser, parser._parseStylesheet.bind(parser), ParseError.RightSquareBracketExpected); + assertError('@unknown-rule (foo) {', parser, parser._parseStylesheet.bind(parser), ParseError.RightCurlyExpected); assertError('@unknown-rule (foo) { .bar {}', parser, parser._parseStylesheet.bind(parser), ParseError.RightCurlyExpected); });