From 125a2cc64c51cb1d5fbfcb7962f8ee750ff7436d Mon Sep 17 00:00:00 2001 From: Landon Abney Date: Mon, 17 Jul 2017 15:39:11 -0700 Subject: [PATCH] Migrate to Linter v2 API Move to the v2 Linter API, allowing for cleaner messages and no need for translation of the messages on Linter's part. --- lib/helpers.js | 108 +++++++++++++++++----------------- lib/index.js | 10 ++-- package.json | 7 +-- spec/linter-stylelint-spec.js | 83 ++++++++++++-------------- 4 files changed, 99 insertions(+), 109 deletions(-) diff --git a/lib/helpers.js b/lib/helpers.js index 49b06bbc..fd7d282c 100644 --- a/lib/helpers.js +++ b/lib/helpers.js @@ -3,7 +3,6 @@ import { dirname } from 'path'; import stylelint from 'stylelint'; import assignDeep from 'assign-deep'; -import escapeHTML from 'escape-html'; import { generateRange } from 'atom-linter'; import presetConfig from 'stylelint-config-standard'; @@ -30,48 +29,19 @@ export function endMeasure(baseName) { } export function createRange(editor, data) { - if (!Object.hasOwnProperty.call(data, 'line') && !Object.hasOwnProperty.call(data, 'column')) { + if (!data || + (!Object.hasOwnProperty.call(data, 'line') && !Object.hasOwnProperty.call(data, 'column')) + ) { // data.line & data.column might be undefined for non-fatal invalid rules, // e.g.: "block-no-empty": "foo" - // Return `false` so Linter will ignore the range - return false; + // Return a range encompassing the first line of the file + return generateRange(editor); } return generateRange(editor, data.line - 1, data.column - 1); } -export function generateHTMLMessage(message) { - if (!message.rule || message.rule === 'CssSyntaxError') { - return escapeHTML(message.text); - } - - const ruleParts = message.rule.split('/'); - let url; - - if (ruleParts.length === 1) { - // Core rule - url = `http://stylelint.io/user-guide/rules/${ruleParts[0]}`; - } else { - // Plugin rule - const pluginName = ruleParts[0]; - // const ruleName = ruleParts[1]; - - switch (pluginName) { - case 'plugin': - url = 'https://github.com/AtomLinter/linter-stylelint/tree/master/docs/noRuleNamespace.md'; - break; - default: - url = 'https://github.com/AtomLinter/linter-stylelint/tree/master/docs/linkingNewRule.md'; - } - } - - // Escape any HTML in the message, and replace the rule ID with a link - return escapeHTML(message.text).replace( - `(${message.rule})`, `(${message.rule})` - ); -} - -export const parseResults = (editor, results, filePath, showIgnored) => { +const parseResults = (editor, results, filePath, showIgnored) => { startMeasure('linter-stylelint: Parsing results'); if (!results) { endMeasure('linter-stylelint: Parsing results'); @@ -80,38 +50,67 @@ export const parseResults = (editor, results, filePath, showIgnored) => { } const invalidOptions = results.invalidOptionWarnings.map(msg => ({ - type: 'Error', severity: 'error', - text: msg.text, - filePath + excerpt: msg.text, + location: { + file: filePath, + position: createRange(editor) + } })); const warnings = results.warnings.map((warning) => { // Stylelint only allows 'error' and 'warning' as severity values const severity = !warning.severity || warning.severity === 'error' ? 'Error' : 'Warning'; - return { - type: severity, + const message = { severity: severity.toLowerCase(), - html: generateHTMLMessage(warning), - filePath, - range: createRange(editor, warning) + excerpt: warning.text, + location: { + file: filePath, + position: createRange(editor, warning) + } }; + + const ruleParts = warning.rule.split('/'); + if (ruleParts.length === 1) { + // Core rule + message.url = `http://stylelint.io/user-guide/rules/${ruleParts[0]}`; + } else { + // Plugin rule + const pluginName = ruleParts[0]; + // const ruleName = ruleParts[1]; + + const linterStylelintURL = 'https://github.com/AtomLinter/linter-stylelint/tree/master/docs'; + switch (pluginName) { + case 'plugin': + message.url = `${linterStylelintURL}/noRuleNamespace.md`; + break; + default: + message.url = `${linterStylelintURL}/linkingNewRule.md`; + } + } + + return message; }); const deprecations = results.deprecations.map(deprecation => ({ - type: 'Warning', severity: 'warning', - html: `${escapeHTML(deprecation.text)} (reference)`, - filePath + excerpt: deprecation.text, + url: deprecation.reference, + location: { + file: filePath, + position: createRange(editor) + } })); const ignored = []; if (showIgnored && results.ignored) { ignored.push({ - type: 'Warning', severity: 'warning', - text: 'This file is ignored', - filePath + excerpt: 'This file is ignored', + location: { + file: filePath, + position: createRange(editor) + } }); } @@ -137,11 +136,12 @@ export const runStylelint = async (editor, stylelintOptions, filePath, settings) if (error.line) { endMeasure('linter-stylelint: Lint'); return [{ - type: 'Error', severity: 'error', - text: error.reason || error.message, - filePath, - range: createRange(editor, error) + excerpt: error.reason || error.message, + location: { + file: filePath, + position: createRange(editor, error) + } }]; } diff --git a/lib/index.js b/lib/index.js index ef599d1c..b052c90b 100644 --- a/lib/index.js +++ b/lib/index.js @@ -86,7 +86,7 @@ export default { name: 'stylelint', grammarScopes: this.baseScopes, scope: 'file', - lintOnFly: true, + lintsOnChange: true, lint: async (editor) => { // Force the dependencies to load if they haven't already loadDeps(); @@ -178,10 +178,12 @@ export default { helpers.endMeasure('linter-stylelint: Lint'); if (this.showIgnored) { return [{ - type: 'Warning', severity: 'warning', - text: 'This file is ignored', - filePath + excerpt: 'This file is ignored', + location: { + file: filePath, + position: helpers.createRange(editor) + } }]; } return []; diff --git a/package.json b/package.json index 1275f521..77104ad1 100644 --- a/package.json +++ b/package.json @@ -53,8 +53,7 @@ "dependencies": { "assign-deep": "^0.4.5", "atom-linter": "^10.0.0", - "atom-package-deps": "^4.0.1", - "escape-html": "^1.0.3", + "atom-package-deps": "^4.6.0", "stylelint": "8.0.0", "stylelint-config-standard": "^17.0.0" }, @@ -90,12 +89,12 @@ } }, "package-deps": [ - "linter" + "linter:2.0.0" ], "providedServices": { "linter": { "versions": { - "1.0.0": "provideLinter" + "2.0.0": "provideLinter" } } } diff --git a/spec/linter-stylelint-spec.js b/spec/linter-stylelint-spec.js index 66b30c6c..14cde669 100644 --- a/spec/linter-stylelint-spec.js +++ b/spec/linter-stylelint-spec.js @@ -9,7 +9,8 @@ const configStandardPath = path.join(fixtures, 'bad', 'stylelint-config-standard const warningPath = path.join(fixtures, 'warn', 'warn.css'); const invalidRulePath = path.join(fixtures, 'invalid-rule', 'styles.css'); -const blockNoEmpty = 'Unexpected empty block (block-no-empty)'; +const blockNoEmpty = 'Unexpected empty block (block-no-empty)'; +const blockNoEmptyUrl = 'http://stylelint.io/user-guide/rules/block-no-empty'; describe('The stylelint provider for Linter', () => { const lint = require('../lib/index.js').provideLinter().lint; @@ -28,12 +29,11 @@ describe('The stylelint provider for Linter', () => { expect(messages.length).toBeGreaterThan(0); // test only the first error - expect(messages[0].type).toBe('Error'); expect(messages[0].severity).toBe('error'); - expect(messages[0].text).not.toBeDefined(); - expect(messages[0].html).toBe(blockNoEmpty); - expect(messages[0].filePath).toBe(configStandardPath); - expect(messages[0].range).toEqual([[0, 5], [0, 7]]); + expect(messages[0].excerpt).toBe(blockNoEmpty); + expect(messages[0].url).toBe(blockNoEmptyUrl); + expect(messages[0].location.file).toBe(configStandardPath); + expect(messages[0].location.position).toEqual([[0, 5], [0, 7]]); }); it('reports rules set as warnings as a Warning', async () => { @@ -43,12 +43,11 @@ describe('The stylelint provider for Linter', () => { expect(messages.length).toBeGreaterThan(0); // test only the first error - expect(messages[0].type).toBe('Warning'); expect(messages[0].severity).toBe('warning'); - expect(messages[0].text).not.toBeDefined(); - expect(messages[0].html).toBe(blockNoEmpty); - expect(messages[0].filePath).toMatch(/.+warn\.css$/); - expect(messages[0].range).toEqual([[0, 5], [0, 7]]); + expect(messages[0].excerpt).toBe(blockNoEmpty); + expect(messages[0].url).toBe(blockNoEmptyUrl); + expect(messages[0].location.file).toMatch(/.+warn\.css$/); + expect(messages[0].location.position).toEqual([[0, 5], [0, 7]]); }); it('finds nothing wrong with a valid file', async () => { @@ -64,12 +63,10 @@ describe('The stylelint provider for Linter', () => { const messages = await lint(editor); expect(messages.length).toBe(1); - expect(messages[0].type).toBe('Error'); expect(messages[0].severity).toBe('error'); - expect(messages[0].text).not.toBeDefined(); - expect(messages[0].html).toBe('Unknown word (CssSyntaxError)'); - expect(messages[0].filePath).toBe(invalidPath); - expect(messages[0].range).toEqual([[0, 0], [0, 3]]); + expect(messages[0].excerpt).toBe('Unknown word (CssSyntaxError)'); + expect(messages[0].location.file).toBe(invalidPath); + expect(messages[0].location.position).toEqual([[0, 0], [0, 3]]); }); it('shows an error on non-fatal stylelint runtime error', async () => { @@ -78,12 +75,10 @@ describe('The stylelint provider for Linter', () => { const messages = await lint(editor); expect(messages.length).toBe(1); - expect(messages[0].type).toBe('Error'); expect(messages[0].severity).toBe('error'); - expect(messages[0].text).toBe(text); - expect(messages[0].html).not.toBeDefined(); - expect(messages[0].filePath).toBe(invalidRulePath); - expect(messages[0].range).not.toBeDefined(); + expect(messages[0].excerpt).toBe(text); + expect(messages[0].location.file).toBe(invalidRulePath); + expect(messages[0].location.position).toEqual([[0, 0], [0, 6]]); }); it('shows an error notification for a fatal stylelint runtime error', async () => { @@ -138,12 +133,10 @@ describe('The stylelint provider for Linter', () => { const messages = await lint(editor); expect(messages.length).toBe(1); - expect(messages[0].type).toBe('Warning'); expect(messages[0].severity).toBe('warning'); - expect(messages[0].text).toBe('This file is ignored'); - expect(messages[0].html).not.toBeDefined(); - expect(messages[0].filePath).toBe(ignorePath); - expect(messages[0].range).not.toBeDefined(); + expect(messages[0].excerpt).toBe('This file is ignored'); + expect(messages[0].location.file).toBe(ignorePath); + expect(messages[0].location.position).toEqual([[0, 0], [0, 7]]); }); it("doesn't show a message when not asked to", async () => { @@ -165,12 +158,11 @@ describe('The stylelint provider for Linter', () => { expect(messages.length).toBeGreaterThan(0); // test only the first error - expect(messages[0].type).toBe('Warning'); expect(messages[0].severity).toBe('warning'); - expect(messages[0].text).not.toBeDefined(); - expect(messages[0].html).toBe(blockNoEmpty); - expect(messages[0].filePath).toBe(warningPath); - expect(messages[0].range).toEqual([[0, 5], [0, 7]]); + expect(messages[0].excerpt).toBe(blockNoEmpty); + expect(messages[0].url).toBe(blockNoEmptyUrl); + expect(messages[0].location.file).toBe(warningPath); + expect(messages[0].location.position).toEqual([[0, 5], [0, 7]]); }); describe('works with Less files and', () => { @@ -188,12 +180,11 @@ describe('The stylelint provider for Linter', () => { expect(messages.length).toBeGreaterThan(0); // test only the first error - expect(messages[0].type).toBe('Error'); expect(messages[0].severity).toBe('error'); - expect(messages[0].text).not.toBeDefined(); - expect(messages[0].html).toBe(blockNoEmpty); - expect(messages[0].filePath).toBe(badLess); - expect(messages[0].range).toEqual([[0, 5], [0, 7]]); + expect(messages[0].excerpt).toBe(blockNoEmpty); + expect(messages[0].url).toBe(blockNoEmptyUrl); + expect(messages[0].location.file).toBe(badLess); + expect(messages[0].location.position).toEqual([[0, 5], [0, 7]]); }); it('finds nothing wrong with a valid file', async () => { @@ -217,12 +208,11 @@ describe('The stylelint provider for Linter', () => { expect(messages.length).toBeGreaterThan(0); // test only the first error - expect(messages[0].type).toBe('Error'); expect(messages[0].severity).toBe('error'); - expect(messages[0].text).not.toBeDefined(); - expect(messages[0].html).toBe(blockNoEmpty); - expect(messages[0].filePath).toBe(issuesPostCSS); - expect(messages[0].range).toEqual([[0, 5], [0, 7]]); + expect(messages[0].excerpt).toBe(blockNoEmpty); + expect(messages[0].url).toBe(blockNoEmptyUrl); + expect(messages[0].location.file).toBe(issuesPostCSS); + expect(messages[0].location.position).toEqual([[0, 5], [0, 7]]); }); it('finds nothing wrong with a valid file', async () => { @@ -241,15 +231,14 @@ describe('The stylelint provider for Linter', () => { }); it('shows lint messages when found', async () => { - const nlzMessage = 'Expected a leading zero (number-leading-zero)'; const editor = await atom.workspace.open(badSugarSS); const messages = await lint(editor); - expect(messages[0].type).toBe('Error'); + expect(messages[0].severity).toBe('error'); - expect(messages[0].text).not.toBeDefined(); - expect(messages[0].html).toBe(nlzMessage); - expect(messages[0].filePath).toBe(badSugarSS); - expect(messages[0].range).toEqual([[1, 38], [1, 40]]); + expect(messages[0].excerpt).toBe('Expected a leading zero (number-leading-zero)'); + expect(messages[0].url).toBe('http://stylelint.io/user-guide/rules/number-leading-zero'); + expect(messages[0].location.file).toBe(badSugarSS); + expect(messages[0].location.position).toEqual([[1, 38], [1, 40]]); }); it('finds nothing wrong with a valid file', async () => {