From 0330fa1fd8f2a968914b05137f5c39d500fcd624 Mon Sep 17 00:00:00 2001 From: bellzebu Date: Wed, 30 Jan 2019 22:36:58 +0100 Subject: [PATCH 1/8] Support v2 Linter API --- lib/main.js | 17 ++++++++++------- package.json | 4 ++-- spec/linter-haml-spec.js | 18 +++++++++--------- 3 files changed, 21 insertions(+), 18 deletions(-) diff --git a/lib/main.js b/lib/main.js index 507cc66..e1ee83a 100644 --- a/lib/main.js +++ b/lib/main.js @@ -38,7 +38,7 @@ export default { name: 'HAML-Lint', grammarScopes: ['text.haml'], scope: 'file', - lintOnFly: false, + lintsOnChange: false, lint: async (textEditor) => { const filePath = textEditor.getPath(); const text = textEditor.getText(); @@ -74,15 +74,18 @@ export default { const messages = []; let match = regex.exec(output); while (match !== null) { - const type = match[2] === 'W' ? 'Warning' : 'Error'; + const severity = match[2] === 'W' ? 'warning' : 'error'; const line = Number.parseInt(match[1], 10) - 1; const ruleName = escapeHtml(match[3]); - const message = escapeHtml(match[4]); + const excerpt = escapeHtml(match[4]); messages.push({ - type, - filePath, - range: helpers.generateRange(textEditor, line), - html: `${ruleName}: ${message}`, + url: `${urlBase}#${ruleName.toLowerCase()}`, + severity, + excerpt: `${ruleName}: ${excerpt}`, + location: { + file: filePath, + position: helpers.generateRange(textEditor, line), + }, }); match = regex.exec(output); diff --git a/package.json b/package.json index cd8d5a4..282a1cd 100644 --- a/package.json +++ b/package.json @@ -27,13 +27,13 @@ "escape-html": "^1.0.3" }, "package-deps": [ - "linter", + "linter:2.0.0", "language-haml" ], "providedServices": { "linter": { "versions": { - "1.0.0": "provideLinter" + "2.0.0": "provideLinter" } } }, diff --git a/spec/linter-haml-spec.js b/spec/linter-haml-spec.js index 2b1414e..b719a5c 100644 --- a/spec/linter-haml-spec.js +++ b/spec/linter-haml-spec.js @@ -23,17 +23,17 @@ describe('The haml-lint provider for Linter', () => { it('checks a file with issues', async () => { const editor = await atom.workspace.open(cawsvpath); const messages = await lint(editor); - const messageText = 'ClassAttributeWithStaticValue: ' + - 'Avoid defining `class` in attributes hash for static class names'; + const url = 'https://github.com/brigade/haml-lint/blob/master/lib/haml_lint/linter/README.md' + + '#classattributewithstaticvalue'; + const excerpt = 'ClassAttributeWithStaticValue: Avoid defining `class` in attributes hash for static class names'; expect(messages.length).toBe(1); - expect(messages[0].type).toBe('Warning'); - expect(messages[0].text).not.toBeDefined(); - expect(messages[0].html).toBe(messageText); - expect(messages[0].filePath).toBe(cawsvpath); - expect(messages[0].range).toEqual([[0, 0], [0, 23]]); + expect(messages[0].severity).toBe('warning'); + expect(messages[0].excerpt).toBe(excerpt); + expect(messages[0].description).not.toBeDefined(); + expect(messages[0].url).toBe(url); + expect(messages[0].location.file).toBe(cawsvpath); + expect(messages[0].location.position).toEqual([[0, 0], [0, 23]]); }); it('finds nothing wrong with a valid file', async () => { From c1529b1b6861a8ab900066fd168b65c1c20ed38f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=ADctor=20Zamanillo?= Date: Thu, 31 Jan 2019 11:06:15 +0100 Subject: [PATCH 2/8] Fixed #117 We always handle two types of error, exec errors and program stream errors, in the first case we show a notification with the error related information, in the second case we check the exit code and text looking for warnings, if found we will show a warning notification with the warning related information, else an error notification. A new configuration option can suppress the warning notifications to avoid notification spam to the user. This will solve CI errors. --- lib/main.js | 30 ++++++++++++++++++++++++++++-- package.json | 7 ++++++- 2 files changed, 34 insertions(+), 3 deletions(-) diff --git a/lib/main.js b/lib/main.js index e1ee83a..bb2a76e 100644 --- a/lib/main.js +++ b/lib/main.js @@ -10,10 +10,15 @@ import { dirname } from 'path'; // Some internal variables let executablePath; let globalHamlLintYmlFile; +let suppressWarnings; const regex = /.+?:(\d+) \[(W|E)] (\w+): (.+)/g; const urlBase = 'https://github.com/brigade/haml-lint/blob/master/lib/haml_lint/linter/README.md'; +const showErrorNotification = (text, description) => { + atom.notifications.addError(text, { description }); +}; + export default { activate() { require('atom-package-deps').install('linter-haml'); @@ -26,6 +31,9 @@ export default { atom.config.observe('linter-haml.globalHamlLintYmlFile', (value) => { globalHamlLintYmlFile = value; }), + atom.config.observe('linter-haml.suppressWarnings', (value) => { + suppressWarnings = value; + }), ); }, @@ -45,7 +53,9 @@ export default { const options = { cwd: dirname(filePath), + stream: 'both', ignoreExitCode: true, + uniqueKey: `linter-haml::${filePath}`, }; const parameters = []; @@ -60,7 +70,23 @@ export default { // Add the file to be linted parameters.push(filePath); - const output = await helpers.exec(executablePath, parameters, options); + let output; + try { + output = await helpers.exec(executablePath, parameters, options); + } catch (e) { + showErrorNotification('HAML-Lint: Unexpected error', e.message); + return null; + } + + if (output.exitCode !== 0) { + if (!suppressWarnings && output.stderr.toLowerCase().startsWith('warning')) { + atom.notifications.addWarning('HAML-Lint: Software error', { + description: output.stderr, + }); + } + } else { + showErrorNotification('HAML-Lint: Unexpected error', output.stderr); + } if (textEditor.getText() !== text) { // eslint-disable-next-line no-console @@ -72,7 +98,7 @@ export default { } const messages = []; - let match = regex.exec(output); + let match = regex.exec(output.stdout); while (match !== null) { const severity = match[2] === 'W' ? 'warning' : 'error'; const line = Number.parseInt(match[1], 10) - 1; diff --git a/package.json b/package.json index 282a1cd..05bfa06 100644 --- a/package.json +++ b/package.json @@ -12,13 +12,18 @@ "configSchema": { "executablePath": { "default": "haml-lint", - "description": "Path to haml-lint executable", + "description": "This is the absolute path to your `haml-lint` command. You may need to run `which haml-lint` or `rbenv which haml-lint` to find this.", "type": "string" }, "globalHamlLintYmlFile": { "default": "", "description": "Full path to a global Haml lint file, if no other is found", "type": "string" + }, + "suppressWarnings": { + "default": false, + "description": "This suppress the haml-lint executable warning notifications.", + "type": "boolean" } }, "dependencies": { From 68cde6ccc6143c300c2eb7e0e969c0f0d6c696c5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=ADctor=20Zamanillo?= Date: Thu, 31 Jan 2019 13:39:04 +0100 Subject: [PATCH 3/8] Fixed matching for multiple lines --- lib/main.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/main.js b/lib/main.js index bb2a76e..e97f2f0 100644 --- a/lib/main.js +++ b/lib/main.js @@ -114,7 +114,7 @@ export default { }, }); - match = regex.exec(output); + match = regex.exec(output.stdout); } return messages; }, From 5170dbc49d88db3907e882f75a6ec87222d09066 Mon Sep 17 00:00:00 2001 From: bellzebu Date: Fri, 1 Feb 2019 19:22:40 +0100 Subject: [PATCH 4/8] Removed not needed html escape --- lib/main.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/main.js b/lib/main.js index e97f2f0..abc5faa 100644 --- a/lib/main.js +++ b/lib/main.js @@ -103,7 +103,7 @@ export default { const severity = match[2] === 'W' ? 'warning' : 'error'; const line = Number.parseInt(match[1], 10) - 1; const ruleName = escapeHtml(match[3]); - const excerpt = escapeHtml(match[4]); + const excerpt = match[4]; messages.push({ url: `${urlBase}#${ruleName.toLowerCase()}`, severity, From da10a39ee4cd81aee748db5ced82adba939a660e Mon Sep 17 00:00:00 2001 From: bellzebu Date: Fri, 1 Feb 2019 21:07:53 +0100 Subject: [PATCH 5/8] First line message on haml-lint output warnings suppressWarnings config. option and warning notification removed. If the message prevents other results from being returned it should be an Atom error notification, if other results can still be obtained from the linter then it probably should just be converted into a message on the entire first line --- lib/main.js | 28 ++++++++++++++++------------ package.json | 5 ----- 2 files changed, 16 insertions(+), 17 deletions(-) diff --git a/lib/main.js b/lib/main.js index abc5faa..d170cf4 100644 --- a/lib/main.js +++ b/lib/main.js @@ -10,12 +10,12 @@ import { dirname } from 'path'; // Some internal variables let executablePath; let globalHamlLintYmlFile; -let suppressWarnings; +const warning = 'warning'; const regex = /.+?:(\d+) \[(W|E)] (\w+): (.+)/g; const urlBase = 'https://github.com/brigade/haml-lint/blob/master/lib/haml_lint/linter/README.md'; -const showErrorNotification = (text, description) => { +const showErrorNotification = (text = 'HAML-Lint: Unexpected error', description) => { atom.notifications.addError(text, { description }); }; @@ -31,9 +31,6 @@ export default { atom.config.observe('linter-haml.globalHamlLintYmlFile', (value) => { globalHamlLintYmlFile = value; }), - atom.config.observe('linter-haml.suppressWarnings', (value) => { - suppressWarnings = value; - }), ); }, @@ -74,18 +71,26 @@ export default { try { output = await helpers.exec(executablePath, parameters, options); } catch (e) { - showErrorNotification('HAML-Lint: Unexpected error', e.message); + showErrorNotification(e.message); return null; } + const messages = []; + if (output.exitCode !== 0) { - if (!suppressWarnings && output.stderr.toLowerCase().startsWith('warning')) { - atom.notifications.addWarning('HAML-Lint: Software error', { - description: output.stderr, + if (output.stderr.toLowerCase().startsWith(warning)) { + messages.push({ + severity: warning, + excerpt: `haml-lint: ${output.stderr}`, + location: { + file: filePath, + // first line of the file + position: [[0, 0], [0, Infinity]], + }, }); } } else { - showErrorNotification('HAML-Lint: Unexpected error', output.stderr); + showErrorNotification(output.stderr); } if (textEditor.getText() !== text) { @@ -97,10 +102,9 @@ export default { return null; } - const messages = []; let match = regex.exec(output.stdout); while (match !== null) { - const severity = match[2] === 'W' ? 'warning' : 'error'; + const severity = match[2] === 'W' ? warning : 'error'; const line = Number.parseInt(match[1], 10) - 1; const ruleName = escapeHtml(match[3]); const excerpt = match[4]; diff --git a/package.json b/package.json index 05bfa06..0fab2e9 100644 --- a/package.json +++ b/package.json @@ -19,11 +19,6 @@ "default": "", "description": "Full path to a global Haml lint file, if no other is found", "type": "string" - }, - "suppressWarnings": { - "default": false, - "description": "This suppress the haml-lint executable warning notifications.", - "type": "boolean" } }, "dependencies": { From 418d5b4f80421d729da92bb4fe96be038a49434a Mon Sep 17 00:00:00 2001 From: bellzebu Date: Fri, 1 Feb 2019 22:04:04 +0100 Subject: [PATCH 6/8] Bad condition Error notification if not exit code 0 and is not a warning --- lib/main.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/main.js b/lib/main.js index d170cf4..86f60d3 100644 --- a/lib/main.js +++ b/lib/main.js @@ -88,9 +88,9 @@ export default { position: [[0, 0], [0, Infinity]], }, }); + } else { + showErrorNotification(output.stderr); } - } else { - showErrorNotification(output.stderr); } if (textEditor.getText() !== text) { From 0dd4f6ef8e2dd045e04da6ffe62da85958e10c63 Mon Sep 17 00:00:00 2001 From: bellzebu Date: Mon, 4 Feb 2019 20:44:17 +0100 Subject: [PATCH 7/8] Old compilant syntax ruby version support in specs Conditional validation to pass tests with old compilant syntax ruby versions --- spec/linter-haml-spec.js | 32 +++++++++++++++++++++++++------- 1 file changed, 25 insertions(+), 7 deletions(-) diff --git a/spec/linter-haml-spec.js b/spec/linter-haml-spec.js index b719a5c..24c31bd 100644 --- a/spec/linter-haml-spec.js +++ b/spec/linter-haml-spec.js @@ -27,13 +27,31 @@ describe('The haml-lint provider for Linter', () => { '#classattributewithstaticvalue'; const excerpt = 'ClassAttributeWithStaticValue: Avoid defining `class` in attributes hash for static class names'; - expect(messages.length).toBe(1); - expect(messages[0].severity).toBe('warning'); - expect(messages[0].excerpt).toBe(excerpt); - expect(messages[0].description).not.toBeDefined(); - expect(messages[0].url).toBe(url); - expect(messages[0].location.file).toBe(cawsvpath); - expect(messages[0].location.position).toEqual([[0, 0], [0, 23]]); + const normalWarningExpects = (message) => { + expect(message.severity).toBe('warning'); + expect(message.excerpt).toBe(excerpt); + expect(message.description).not.toBeDefined(); + expect(message.url).toBe(url); + expect(message.location.file).toBe(cawsvpath); + expect(message.location.position).toEqual([[0, 0], [0, 23]]); + }; + + // no old compilant syntax ruby version + expect(messages.length).not.toBeLessThan(1); + // old compilant syntax ruby version (parser warning + ClassAttributeWithStaticValue warning) + expect(messages.length).not.toBeGreaterThan(2); + + if (messages.length > 1 && messages[0].excerpt.startsWith('haml-lint: warning')) { + expect(messages[0].severity).toBe('warning'); + expect(messages[0].excerpt).toContain('haml-lint: warning'); + expect(messages[0].description).not.toBeDefined(); + expect(messages[0].url).not.toBeDefined(); + expect(messages[0].location.file).toBe(cawsvpath); + expect(messages[0].location.position).toEqual([[0, 0], [0, Infinity]]); + normalWarningExpects(messages[1]); + } else { + normalWarningExpects(messages[0]); + } }); it('finds nothing wrong with a valid file', async () => { From 26c2b6c1ae247f9c7422b0cd46fbd056344b094f Mon Sep 17 00:00:00 2001 From: bellzebu Date: Tue, 5 Feb 2019 23:39:54 +0100 Subject: [PATCH 8/8] null return on unexpected errors --- lib/main.js | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/main.js b/lib/main.js index 86f60d3..2dfc5e7 100644 --- a/lib/main.js +++ b/lib/main.js @@ -90,6 +90,7 @@ export default { }); } else { showErrorNotification(output.stderr); + return null; } }