From 18f12896a3f99c2be30ef76fc389c5adbf749bcd Mon Sep 17 00:00:00 2001 From: Xianming Zhong Date: Sat, 4 Nov 2017 23:54:56 +0800 Subject: [PATCH 1/3] Fix reporting the wrong line numbers --- src/parsers/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/parsers/index.js b/src/parsers/index.js index 390c5ff..8c1744d 100644 --- a/src/parsers/index.js +++ b/src/parsers/index.js @@ -74,7 +74,7 @@ const processStyledComponentsFile = (ast, absolutePath, options) => { extractedCSS.push(stylelintCommentsAdded) sourceMap = Object.assign( sourceMap, - getSourceMap(extractedCSS.join('\n'), wrappedContent, processedNode.loc.start.line) + getSourceMap(extractedCSS.join('\n'), wrappedContent, processedNode.quasi.loc.start.line) ) /** * All queued comments have been added to the file so we don't need to, and actually shouldn't From 9cce4c8b63acc56ce28980843f061c8c5e6f2164 Mon Sep 17 00:00:00 2001 From: Xianming Zhong Date: Sun, 5 Nov 2017 12:20:13 +0800 Subject: [PATCH 2/3] Fix substitution changes line counts --- src/utils/tagged-template-literal.js | 11 +++++++ test/fixtures/interpolations/invalid.js | 7 ++++- test/hard.test.js | 16 ++++++++-- test/interpolations.test.js | 4 +++ test/utils.test.js | 39 +++++++++++++++++++++++++ 5 files changed, 73 insertions(+), 4 deletions(-) diff --git a/src/utils/tagged-template-literal.js b/src/utils/tagged-template-literal.js index e0a902c..b09fbf3 100644 --- a/src/utils/tagged-template-literal.js +++ b/src/utils/tagged-template-literal.js @@ -146,6 +146,17 @@ const interleave = (quasis, expressions, absolutePath) => { */ substitute = '$dummyValue' } + // Make sure substituted by same count of lines + const intentCount = prevText.split('\n').pop().length + const intent = new Array(intentCount).fill(' ').join('') + const targetLines = quasis[i + 1].loc.start.line - quasis[i].loc.end.line + 1 + let currentLines = substitute.split('\n').length + while (currentLines < targetLines) { + substitute += `\n${intent}-styled-mixin${count}: dummyValue;` + count += 1 + currentLines += 1 + } + css += substitute } css += quasis[quasis.length - 1].value.raw diff --git a/test/fixtures/interpolations/invalid.js b/test/fixtures/interpolations/invalid.js index ff772e3..f52c300 100644 --- a/test/fixtures/interpolations/invalid.js +++ b/test/fixtures/interpolations/invalid.js @@ -3,8 +3,13 @@ import styled from 'styled-components' const color = 'red' // ⚠️ BAD INDENTATION ⚠️ -const Button2 = styled.button` +const Button2 = styled.button.attrs({ + type: 'normal' +})` display: block; + ${ + props => props.isHovering && interpolatedStyle + } color: ${color}; background: blue; ` diff --git a/test/hard.test.js b/test/hard.test.js index fe0b649..40c5636 100644 --- a/test/hard.test.js +++ b/test/hard.test.js @@ -102,16 +102,18 @@ describe('hard', () => { expect(data.errored).toEqual(true) }) - it('should have five warning', () => { - expect(data.results[0].warnings.length).toEqual(5) + it('should have seven warning', () => { + expect(data.results[0].warnings.length).toEqual(7) }) - it('should have four warnings about indentation', () => { + it('should have seven warnings about indentation', () => { expect(data.results[0].warnings[0].rule).toEqual('indentation') expect(data.results[0].warnings[1].rule).toEqual('indentation') expect(data.results[0].warnings[2].rule).toEqual('indentation') expect(data.results[0].warnings[3].rule).toEqual('indentation') expect(data.results[0].warnings[4].rule).toEqual('indentation') + expect(data.results[0].warnings[5].rule).toEqual('indentation') + expect(data.results[0].warnings[6].rule).toEqual('indentation') }) it('should have a warning in line 5', () => { @@ -133,5 +135,13 @@ describe('hard', () => { it('should have a warning in line 35', () => { expect(data.results[0].warnings[4].line).toEqual(35) }) + + it('should have a warning in line 36', () => { + expect(data.results[0].warnings[5].line).toEqual(36) + }) + + it('should have a warning in line 37', () => { + expect(data.results[0].warnings[6].line).toEqual(37) + }) }) }) diff --git a/test/interpolations.test.js b/test/interpolations.test.js index ceb1fcb..59d9b5a 100644 --- a/test/interpolations.test.js +++ b/test/interpolations.test.js @@ -78,6 +78,10 @@ describe('interpolations', () => { it('should have an indentation as the first warning', () => { expect(data.results[0].warnings[0].rule).toEqual('indentation') }) + + it('should have the indentation warning in the right line', () => { + expect(data.results[0].warnings[0].line).toEqual(13) + }) }) describe('complex interpolations', () => { diff --git a/test/utils.test.js b/test/utils.test.js index 2b4514e..373b2e2 100644 --- a/test/utils.test.js +++ b/test/utils.test.js @@ -11,6 +11,15 @@ const fixIndentation = require('../lib/utils/general').fixIndentation const extrapolateShortenedCommand = require('../lib/utils/general').extrapolateShortenedCommand const removeBaseIndentation = require('../lib/utils/general').removeBaseIndentation +const mockLoc = (startLine, endLine) => ({ + start: { + line: startLine + }, + end: { + line: endLine + } +}) + describe('utils', () => { describe('interleave', () => { it('should return the value of the node if no interpolation exists', () => { @@ -28,11 +37,13 @@ describe('utils', () => { it('should variabelize an interpolation', () => { const quasis = [ { + loc: mockLoc(1, 3), value: { raw: '\n display: block;\n color: ' } }, { + loc: mockLoc(3, 5), value: { raw: ';\n background: blue;\n' } @@ -40,6 +51,7 @@ describe('utils', () => { ] const expressions = [ { + loc: mockLoc(3, 3), name: 'color' } ] @@ -51,11 +63,13 @@ describe('utils', () => { it('converts interpolated expressions to dummy mixins', () => { const quasis = [ { + loc: mockLoc(1, 3), value: { raw: '\n display: block;\n ' } }, { + loc: mockLoc(3, 5), value: { raw: '\n background: blue;\n' } @@ -63,6 +77,7 @@ describe('utils', () => { ] const expressions = [ { + loc: mockLoc(3, 3), name: undefined } ] @@ -74,21 +89,25 @@ describe('utils', () => { it('correctly converts several interpolations within a single property', () => { const quasis = [ { + loc: mockLoc(1, 3), value: { raw: '\n display: block;\n border: ' } }, { + loc: mockLoc(3, 3), value: { raw: ' ' } }, { + loc: mockLoc(3, 3), value: { raw: ' ' } }, { + loc: mockLoc(3, 5), value: { raw: ';\n background: blue;\n' } @@ -96,12 +115,15 @@ describe('utils', () => { ] const expressions = [ { + loc: mockLoc(3, 3), name: 'borderWidth' }, { + loc: mockLoc(3, 3), name: 'borderStyle' }, { + loc: mockLoc(3, 3), name: 'color' } ] @@ -113,16 +135,19 @@ describe('utils', () => { it('correctly handles several interpolations in single line of css', () => { const quasis1 = [ { + loc: mockLoc(1, 2), value: { raw: '\n display: ' } }, { + loc: mockLoc(2, 2), value: { raw: '; background: ' } }, { + loc: mockLoc(2, 3), value: { raw: ';\n' } @@ -130,9 +155,11 @@ describe('utils', () => { ] const expressions1 = [ { + loc: mockLoc(2, 2), name: 'display' }, { + loc: mockLoc(2, 2), name: 'background' } ] @@ -142,16 +169,19 @@ describe('utils', () => { const quasis2 = [ { + loc: mockLoc(1, 2), value: { raw: '\n display: ' } }, { + loc: mockLoc(2, 2), value: { raw: '; ' } }, { + loc: mockLoc(2, 3), value: { raw: '\n' } @@ -159,9 +189,11 @@ describe('utils', () => { ] const expressions2 = [ { + loc: mockLoc(2, 2), name: 'display' }, { + loc: mockLoc(3, 3), name: undefined } ] @@ -176,21 +208,25 @@ describe('utils', () => { */ const quasis3 = [ { + loc: mockLoc(1, 2), value: { raw: '\n display: ' } }, { + loc: mockLoc(2, 2), value: { raw: '; ' } }, { + loc: mockLoc(2, 2), value: { raw: ' ' } }, { + loc: mockLoc(2, 3), value: { raw: '\n' } @@ -198,12 +234,15 @@ describe('utils', () => { ] const expressions3 = [ { + loc: mockLoc(2, 2), name: 'display' }, { + loc: mockLoc(2, 2), name: undefined }, { + loc: mockLoc(2, 2), name: undefined } ] From ca14148c806b3614b98625708b05078ff9742a42 Mon Sep 17 00:00:00 2001 From: Xianming Zhong Date: Sat, 2 Dec 2017 13:13:57 +0800 Subject: [PATCH 3/3] Filter indentation warnings generated by interpolations substitution --- src/index.js | 40 ++++++++++++++++++------- src/parsers/index.js | 13 ++++++++ src/utils/tagged-template-literal.js | 5 +--- test/fixtures/interpolations/invalid.js | 3 ++ test/hard.test.js | 16 ++-------- test/interpolations.test.js | 2 +- 6 files changed, 51 insertions(+), 28 deletions(-) diff --git a/src/index.js b/src/index.js index 2c9dea8..0fa2b45 100644 --- a/src/index.js +++ b/src/index.js @@ -1,6 +1,7 @@ const path = require('path') const parse = require('./parsers/index') +const interpolationLinesMap = {} const sourceMapsCorrections = {} const DEFAULT_OPTIONS = { moduleName: 'styled-components' @@ -12,11 +13,15 @@ module.exports = options => ({ try { const absolutePath = path.resolve(process.cwd(), filepath) sourceMapsCorrections[absolutePath] = {} - const { extractedCSS, sourceMap } = parse( + const { extractedCSS, interpolationLines, sourceMap } = parse( input, absolutePath, Object.assign({}, DEFAULT_OPTIONS, options) ) + // Save dummy interpolation lines + interpolationLinesMap[absolutePath] = interpolationLines.concat( + interpolationLinesMap[absolutePath] || [] + ) // Save source location, merging existing corrections with current corrections sourceMapsCorrections[absolutePath] = Object.assign( sourceMapsCorrections[absolutePath], @@ -33,16 +38,31 @@ module.exports = options => ({ }, // Fix sourcemaps result(stylelintResult, filepath) { + const interpolationLines = interpolationLinesMap[filepath] || [] const lineCorrection = sourceMapsCorrections[filepath] - const warnings = stylelintResult.warnings.map(warning => - Object.assign({}, warning, { - // Replace "brace" with "backtick" in warnings, e.g. - // "Unexpected empty line before closing backtick" (instead of "brace") - text: warning.text.replace(/brace/, 'backtick'), - line: lineCorrection[warning.line] - }) - ) + const warnings = stylelintResult.warnings + .filter( + warning => + // Filter indentation warnings generated by interpolations substitution + !( + warning.rule === 'indentation' && + interpolationLines.indexOf(lineCorrection[warning.line]) >= 0 + ) + ) + .map(warning => + Object.assign({}, warning, { + // Replace "brace" with "backtick" in warnings, e.g. + // "Unexpected empty line before closing backtick" (instead of "brace") + text: warning.text.replace(/brace/, 'backtick'), + line: lineCorrection[warning.line] + }) + ) - return Object.assign({}, stylelintResult, { warnings }) + const result = Object.assign({}, stylelintResult, { warnings }) + // Undo `errored` if no warnings with error severity any more + if (result.errored && !warnings.some(warning => warning.severity === 'error')) { + delete result.errored + } + return result } }) diff --git a/src/parsers/index.js b/src/parsers/index.js index 8c1744d..0f04d71 100644 --- a/src/parsers/index.js +++ b/src/parsers/index.js @@ -27,6 +27,7 @@ const processStyledComponentsFile = (ast, absolutePath, options) => { injectGlobal: 'injectGlobal' } let sourceMap = {} + const interpolationLines = [] traverse(ast, { noScope: true, enter({ node }) { @@ -76,6 +77,17 @@ const processStyledComponentsFile = (ast, absolutePath, options) => { sourceMap, getSourceMap(extractedCSS.join('\n'), wrappedContent, processedNode.quasi.loc.start.line) ) + // Save dummy interpolation lines + node.quasi.expressions.forEach((expression, i) => { + // Skip the end line of previous text + // because the indentation is not generated by substitution + let l = node.quasi.quasis[i].loc.end.line + 1 + while (l <= node.quasi.quasis[i + 1].loc.start.line) { + interpolationLines.push(l) + l += 1 + } + }) + /** * All queued comments have been added to the file so we don't need to, and actually shouldn't * add them to the file more than once @@ -86,6 +98,7 @@ const processStyledComponentsFile = (ast, absolutePath, options) => { return { extractedCSS: extractedCSS.join('\n'), + interpolationLines, sourceMap } } diff --git a/src/utils/tagged-template-literal.js b/src/utils/tagged-template-literal.js index b09fbf3..dde1792 100644 --- a/src/utils/tagged-template-literal.js +++ b/src/utils/tagged-template-literal.js @@ -147,13 +147,10 @@ const interleave = (quasis, expressions, absolutePath) => { substitute = '$dummyValue' } // Make sure substituted by same count of lines - const intentCount = prevText.split('\n').pop().length - const intent = new Array(intentCount).fill(' ').join('') const targetLines = quasis[i + 1].loc.start.line - quasis[i].loc.end.line + 1 let currentLines = substitute.split('\n').length while (currentLines < targetLines) { - substitute += `\n${intent}-styled-mixin${count}: dummyValue;` - count += 1 + substitute += '\n/* dummyComment */' currentLines += 1 } diff --git a/test/fixtures/interpolations/invalid.js b/test/fixtures/interpolations/invalid.js index f52c300..dbb3d8f 100644 --- a/test/fixtures/interpolations/invalid.js +++ b/test/fixtures/interpolations/invalid.js @@ -10,6 +10,9 @@ const Button2 = styled.button.attrs({ ${ props => props.isHovering && interpolatedStyle } + position: ${ + props => props.position + }; color: ${color}; background: blue; ` diff --git a/test/hard.test.js b/test/hard.test.js index 40c5636..fe0b649 100644 --- a/test/hard.test.js +++ b/test/hard.test.js @@ -102,18 +102,16 @@ describe('hard', () => { expect(data.errored).toEqual(true) }) - it('should have seven warning', () => { - expect(data.results[0].warnings.length).toEqual(7) + it('should have five warning', () => { + expect(data.results[0].warnings.length).toEqual(5) }) - it('should have seven warnings about indentation', () => { + it('should have four warnings about indentation', () => { expect(data.results[0].warnings[0].rule).toEqual('indentation') expect(data.results[0].warnings[1].rule).toEqual('indentation') expect(data.results[0].warnings[2].rule).toEqual('indentation') expect(data.results[0].warnings[3].rule).toEqual('indentation') expect(data.results[0].warnings[4].rule).toEqual('indentation') - expect(data.results[0].warnings[5].rule).toEqual('indentation') - expect(data.results[0].warnings[6].rule).toEqual('indentation') }) it('should have a warning in line 5', () => { @@ -135,13 +133,5 @@ describe('hard', () => { it('should have a warning in line 35', () => { expect(data.results[0].warnings[4].line).toEqual(35) }) - - it('should have a warning in line 36', () => { - expect(data.results[0].warnings[5].line).toEqual(36) - }) - - it('should have a warning in line 37', () => { - expect(data.results[0].warnings[6].line).toEqual(37) - }) }) }) diff --git a/test/interpolations.test.js b/test/interpolations.test.js index 59d9b5a..d378d44 100644 --- a/test/interpolations.test.js +++ b/test/interpolations.test.js @@ -80,7 +80,7 @@ describe('interpolations', () => { }) it('should have the indentation warning in the right line', () => { - expect(data.results[0].warnings[0].line).toEqual(13) + expect(data.results[0].warnings[0].line).toEqual(16) }) })