Skip to content

Commit

Permalink
- Fixed: findCommentsInRaws giving wrong column number
Browse files Browse the repository at this point in the history
The issue appeared in two cases: 1) CRLF; 2) if there were //-comments before
the comment in question (dues to postcss-scss transforming //-comments into
CSS comments, thus adding extra */ at the end of each)

Applicable to rules `double-slash-comment-inline`,
`double-slash-comment-whitespace-inside`, `operator-no-unspaced`.
  • Loading branch information
dryoma committed Jul 31, 2016
1 parent 25d8923 commit 56bf9c0
Show file tree
Hide file tree
Showing 8 changed files with 195 additions and 49 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

- Fixed: `findCommentsInRaws` fail on parsing selectors like `p:not(.not-p)` (applicable to rules `double-slash-comment-inline`, `double-slash-comment-whitespace-inside`, `operator-no-unspaced`).
- Fixed: 'double-slash-comment-whitespace-inside' false positives on empty comments (e.g. `//`).
- Fixed: `findCommentsInRaws` giving wrong column number (applicable to rules `double-slash-comment-inline`, `double-slash-comment-whitespace-inside`, `operator-no-unspaced`).

# 1.3.1

Expand Down
2 changes: 2 additions & 0 deletions src/rules/double-slash-comment-inline/__tests__/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,8 @@ testRule(rule, {
`,
message: messages.rejected,
description: "Inline comment, inside a ruleset.",
line: 3,
column: 34,
}, {
code: `
a, // Inline comment, between selectors.
Expand Down
2 changes: 1 addition & 1 deletion src/rules/double-slash-comment-inline/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ export default function (expectation, options) {
utils.report({
message,
node: root,
index: comment.start,
index: comment.source.start,
result,
ruleName,
})
Expand Down
94 changes: 71 additions & 23 deletions src/rules/double-slash-comment-whitespace-inside/__tests__/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,36 +13,65 @@ testRule(rule, {

accept: [ {
code: "// Comment with one space",
description: "// Comment with one space.",
description: "{always} // Comment with one space.",
}, {
code: "// Comment with multiple spaces",
description: "// Comment with multiple spaces.",
description: "{always} // Comment with multiple spaces.",
}, {
code: "/// 3-slash comment with space",
description: "/// 3-slash comment with space.",
description: "{always} /// 3-slash comment with space.",
}, {
code: "/*CSS comment*/",
description: "/*CSS comment*/ (ignored)",
description: "{always} /*CSS comment*/ (ignored)",
}, {
code: "///",
description: "`///`.",
description: "{always} `///`.",
}, {
code: "/// ",
description: "`/// `.",
description: "{always} `/// `.",
} ],

reject: [ {
code: "//Comment with no whitespace",
description: "//Comment with no whitespace",
description: "{always} //Comment with no whitespace",
message: messages.expected,
line: 1,
column: 1,
column: 3,
}, {
code: "///3-slash comment with no whitespace",
description: "///3-slash comment with no whitespace",
description: "{always} ///3-slash comment with no whitespace",
message: messages.expected,
line: 1,
column: 1,
column: 4,
}, {
code: `
//
// 1. Address appearance set to searchfield in Safari and Chrome.
// 2. Address box-sizing set to border-box in Safari and Chrome.
//
input[type="search"] {
-webkit-appearance: textfield; // 1
box-sizing: content-box; //2
}
`,
description: "\r\n\r\n\r\n\r\n /// 3-slash comment with space.",
message: messages.expected,
line: 9,
column: 36,
}, {
code: `
input[type="search"] {
-webkit-appearance: textfield; // 1
box-sizing: content-box; //2
}
`,
description: "\r\n\r\n\r\n\r\n /// 3-slash comment with space.",
message: messages.expected,
line: 6,
column: 36,
} ],
})

Expand All @@ -58,38 +87,57 @@ testRule(rule, {

accept: [ {
code: "//Comment with no whitespace",
description: "//Comment with no whitespace",
description: "{never} //Comment with no whitespace",
}, {
code: "///3-slash comment with no whitespace",
description: "///3-slash comment with no whitespace",
description: "{never} ///3-slash comment with no whitespace",
}, {
code: "/* CSS comment */",
description: "/* CSS comment */ (ignored)",
description: "{never} /* CSS comment */ (ignored)",
}, {
code: "///",
description: "`///`.",
description: "{never} `///`.",
}, {
code: "/// ",
description: "`/// `.",
description: "{never} `/// `.",
} ],

reject: [ {
code: "// Comment with one space",
description: "// Comment with one space.",
description: "{never} // Comment with one space.",
message: messages.rejected,
line: 1,
column: 1,
column: 3,
}, {
code: "// Comment with multiple spaces",
description: "// Comment with multiple spaces.",
description: "{never} // Comment with multiple spaces.",
message: messages.rejected,
line: 1,
column: 1,
column: 3,
}, {
code: "/// 3-slash comment with space",
description: "/// 3-slash comment with space.",
code: "\n\n\n\n /// 3-slash comment with space",
description: "\n\n\n\n /// 3-slash comment with space.",
message: messages.rejected,
line: 1,
column: 1,
line: 5,
column: 6,
}, {
code: "\r\n\r\n\r\n\r\n /// 3-slash comment with space",
description: "\r\n\r\n\r\n\r\n /// 3-slash comment with space.",
message: messages.rejected,
line: 5,
column: 6,
}, {
/* eslint-disable no-multiple-empty-lines */
code: `
/// 3-slash comment with space
`,
/* eslint-disable no-multiple-empty-lines */
description: "\r\n\r\n\r\n\r\n /// 3-slash comment with space.",
message: messages.rejected,
line: 5,
column: 10,
} ],
})
2 changes: 1 addition & 1 deletion src/rules/double-slash-comment-whitespace-inside/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ export default function (expectation) {
utils.report({
message,
node: root,
index: comment.start,
index: comment.source.start + comment.raws.startToken.length,
result,
ruleName,
})
Expand Down
2 changes: 1 addition & 1 deletion src/rules/operator-no-unspaced/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ export function calculationOperatorSpaceChecker({ root, result, checker }) {
// Checking interpolation inside comments
// We have to give up on PostCSS here because it skips some inline comments
findCommentsInRaws(root.source.input.css).forEach(comment => {
const startIndex = comment.start + comment.raws.startToken.length +
const startIndex = comment.source.start + comment.raws.startToken.length +
comment.raws.left.length
if (comment.type !== "css") { return }

Expand Down
109 changes: 95 additions & 14 deletions src/utils/__tests__/findCommentsInRaws.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ test("Various.", t => {
a
{}
b // comment 2
b // there's a trailing whitespace at the end
{
width: /* comment 3 */ 100px;
background: url(http://lol);
Expand All @@ -31,19 +31,40 @@ test("Various.", t => {
t.equal(comments[0].raws.startToken, "/**!")
t.equal(comments[0].raws.endToken, "*/")
t.equal(comments[0].inlineAfter, false)
t.deepEqual(comments[0].source, { start: 7, end: 21 })
t.equal(comments[1].type, "double-slash")
t.equal(comments[1].text, "comment 2")
t.equal(comments[1].start, 55)
t.equal(comments[1].end, 67)
t.equal(comments[1].text, "there's a trailing whitespace at the end")
t.deepEqual(comments[1].source, { start: 55, end: 98 })
t.equal(comments[1].inlineAfter, true)
t.equal(comments[2].inlineBefore, true)
t.equal(comments[2].inlineAfter, true)
t.equal(comments[2].start, 92)
// Here we take into account the fact that postcss-scss transforms
// `// comment` -> `/* comment*/`, thus adding 2 symbols to the end
t.deepEqual(comments[2].source, { start: 125, end: 139 })
})
.catch(logError)
})

test("", t => {
test("//", t => {
t.plan(4)

postcss()
.process(`
//
`, { syntax: scss })
.then(result => {
const css = result.root.source.input.css
const comments = findCommentsInRaws(css)

t.equal(comments.length, 1)
t.equal(comments[0].text, "")
t.equal(comments[0].inlineAfter, false)
t.deepEqual(comments[0].source, { start: 7, end: 8 })
})
.catch(logError)
})

test("// Inline comment, after {.", t => {
t.plan(3)

postcss()
Expand All @@ -63,7 +84,7 @@ test("", t => {
.catch(logError)
})

test("", t => {
test("} // comment", t => {
t.plan(2)

postcss()
Expand Down Expand Up @@ -97,7 +118,7 @@ test("Triple-slash comment", t => {
})

test("Some fancy comment", t => {
t.plan(4)
t.plan(5)

postcss()
.process(`
Expand All @@ -114,6 +135,7 @@ test("Some fancy comment", t => {
t.equal(comments[0].inlineAfter, false, "Inline after something")
t.equal(comments[0].raws.left, `
`, "Raws left.")
t.deepEqual(comments[0].source, { start: 7, end: 189 })
})
.catch(logError)
})
Expand All @@ -139,27 +161,30 @@ test("Another fancy comment", t => {
.catch(logError)
})

test("Another fancy comment", t => {
t.plan(4)
test("Comments inside comments", t => {
t.plan(7)

postcss()
.process(`
/* Text.. /* is that a new comment? */
/* And // this? */
// And /* this */ ?
/* And // this? */
`, { syntax: scss })
.then(result => {
const css = result.root.source.input.css
const comments = findCommentsInRaws(css)
t.equal(comments.length, 3)
t.equal(comments[0].text, "Text.. /* is that a new comment?", "text 1")
t.equal(comments[1].text, "And // this?", "text 2")
t.equal(comments[2].text, "And /* this */ ?", "text 3")
t.deepEqual(comments[0].source, { start: 7, end: 44 })
t.equal(comments[1].text, "And /* this */ ?", "text 3")
t.deepEqual(comments[1].source, { start: 52, end: 70 })
t.equal(comments[2].text, "And // this?", "text 2")
t.deepEqual(comments[2].source, { start: 80, end: 97 })
})
.catch(logError)
})

test("Another fancy comment", t => {
test("No comments, but parsing a selector with ().", t => {
t.plan(1)

postcss()
Expand All @@ -179,3 +204,59 @@ test("Another fancy comment", t => {
})
.catch(logError)
})

test("//-comment, Unix newlines", t => {
t.plan(2)

postcss()
.process("\n // comment \n", { syntax: scss })
.then(result => {
const css = result.root.source.input.css
const comments = findCommentsInRaws(css)
t.equal(comments.length, 1)
t.deepEqual(comments[0].source, { start: 4, end: 14 })
})
.catch(logError)
})

test("CSS comment, Unix newlines", t => {
t.plan(2)

postcss()
.process("\n /* part 1 \n part 2*/ \n", { syntax: scss })
.then(result => {
const css = result.root.source.input.css
const comments = findCommentsInRaws(css)
t.equal(comments.length, 1)
t.deepEqual(comments[0].source, { start: 4, end: 23 })
})
.catch(logError)
})

test("//-comment, Windows-newline", t => {
t.plan(2)

postcss()
.process("\r\n // comment \r\n", { syntax: scss })
.then(result => {
const css = result.root.source.input.css
const comments = findCommentsInRaws(css)
t.equal(comments.length, 1)
t.deepEqual(comments[0].source, { start: 5, end: 15 })
})
.catch(logError)
})

test("CSS comment, Windows newlines", t => {
t.plan(2)

postcss()
.process("\r\n /* part 1 \r\n part 2*/ \r\n", { syntax: scss })
.then(result => {
const css = result.root.source.input.css
const comments = findCommentsInRaws(css)
t.equal(comments.length, 1)
t.deepEqual(comments[0].source, { start: 5, end: 25 })
})
.catch(logError)
})
Loading

0 comments on commit 56bf9c0

Please sign in to comment.