From faa467bef1df34c2b95c2055c356651582ada5b5 Mon Sep 17 00:00:00 2001 From: Fabio Beltramini Date: Tue, 11 Jul 2023 18:45:32 -0400 Subject: [PATCH 01/10] Create outputters directory, extract and migrate markdown outputter --- __tests__/template-functions.test.js | 2 +- index.js | 24 ++------------ lib/outputters/index.js | 5 +++ lib/outputters/markdown.js | 32 +++++++++++++++++++ .../templating}/template-functions.js | 0 lib/{ => outputters/templating}/templates.js | 0 .../templating}/templates/developer.jst | 0 .../templating}/templates/issues.jst | 0 lib/tracking.js | 2 +- 9 files changed, 41 insertions(+), 24 deletions(-) create mode 100644 lib/outputters/index.js create mode 100644 lib/outputters/markdown.js rename lib/{ => outputters/templating}/template-functions.js (100%) rename lib/{ => outputters/templating}/templates.js (100%) rename lib/{ => outputters/templating}/templates/developer.jst (100%) rename lib/{ => outputters/templating}/templates/issues.jst (100%) diff --git a/__tests__/template-functions.test.js b/__tests__/template-functions.test.js index 4d0134b..2bd0133 100644 --- a/__tests__/template-functions.test.js +++ b/__tests__/template-functions.test.js @@ -1,5 +1,5 @@ /* Copyright (c) 2018 Looker Data Sciences, Inc. See https://github.com/looker-open-source/look-at-me-sideways/blob/master/LICENSE.txt */ -const {groupBy} = require('../lib/template-functions.js'); +const {groupBy} = require('../lib/outputters/templating/template-functions.js'); describe('Template Functions', () => { describe('groupBy', () => { diff --git a/index.js b/index.js index f6421f1..f1e7927 100755 --- a/index.js +++ b/index.js @@ -219,6 +219,7 @@ module.exports = async function( } // Output + const outputters = require('./lib/outputters/index.js'); let errors = messages.filter((msg) => { return msg.level === 'error' && !msg.exempt; }); @@ -232,7 +233,7 @@ module.exports = async function( case '': break; case 'markdown': { const {dateOutput} = options; - await outputMarkdown(messages, {dateOutput}); + await outputters.markdown(messages, {dateOutput,console}); break; } case 'markdown-developer': @@ -308,27 +309,6 @@ module.exports = async function( fs.writeFileSync('results.json', json, 'utf8'); } - /** - * Output markdown.md, primarily for reporting in native Looker IDE. - * - * @param {array} messages Array of messages - * @param {object} options Options - * @param {object} options.dateOutput Whether to include a timestamp in the output. Including a timestamp may not be desireable when committing the markdown file to your repo as it creates otherwise unneccessary changes. - * @return {void} - */ - async function outputMarkdown(messages, {dateOutput}) { - console.log('Writing issues.md...'); - const asyncTemplates = require('./lib/templates.js'); - const templates = await asyncTemplates; - const jobURL = process.env && process.env.BUILD_URL || undefined; - fs.writeFileSync('issues.md', templates.issues({ - messages, - jobURL, - dateOutput, - }).replace(/\n\t+/g, '\n')); - console.log('> issues.md done'); - } - /** * Output developer.md, which may help developers navigate the project. Available for backwards compatibility, but generally less used. * diff --git a/lib/outputters/index.js b/lib/outputters/index.js new file mode 100644 index 0000000..55a4f27 --- /dev/null +++ b/lib/outputters/index.js @@ -0,0 +1,5 @@ +const outputters = { + markdown: require("./markdown.js") +}; + +module.exports = outputters; \ No newline at end of file diff --git a/lib/outputters/markdown.js b/lib/outputters/markdown.js new file mode 100644 index 0000000..0c9d6bd --- /dev/null +++ b/lib/outputters/markdown.js @@ -0,0 +1,32 @@ +const defaultConsole = console; +const defaultFs = require('node:fs/promises') + +/** + * Output markdown.md, primarily for reporting in native Looker IDE. + * + * @param {array} messages Array of messages + * @param {object} options Options + * @param {boolean} options.dateOutput Whether to include a timestamp in the output. Including a timestamp may not be desireable when committing the markdown file to your repo as it creates otherwise unneccessary changes. + * @param {object} options.console I/O override for console + * @param {object} options.fs I/O override for fs (filesystem) + * @return {void} + */ + +async function markdown(messages, { + dateOutput, + console = defaultConsole, + fs = defaultFs + }) { + console.log('Writing issues.md...'); + const asyncTemplates = require('./templating/templates.js'); + const templates = await asyncTemplates; + const jobURL = process.env && process.env.BUILD_URL || undefined; + await fs.writeFile('issues.md', templates.issues({ + messages, + jobURL, + dateOutput, + }).replace(/\n\t+/g, '\n')); + console.log('> issues.md done'); +} + +module.exports = markdown \ No newline at end of file diff --git a/lib/template-functions.js b/lib/outputters/templating/template-functions.js similarity index 100% rename from lib/template-functions.js rename to lib/outputters/templating/template-functions.js diff --git a/lib/templates.js b/lib/outputters/templating/templates.js similarity index 100% rename from lib/templates.js rename to lib/outputters/templating/templates.js diff --git a/lib/templates/developer.jst b/lib/outputters/templating/templates/developer.jst similarity index 100% rename from lib/templates/developer.jst rename to lib/outputters/templating/templates/developer.jst diff --git a/lib/templates/issues.jst b/lib/outputters/templating/templates/issues.jst similarity index 100% rename from lib/templates/issues.jst rename to lib/outputters/templating/templates/issues.jst diff --git a/lib/tracking.js b/lib/tracking.js index 36814a3..ca078c6 100644 --- a/lib/tracking.js +++ b/lib/tracking.js @@ -2,7 +2,7 @@ const defaultProcess = process; const defaultConsole = console; const pjson = require('../package.json'); -const {groupBy} = require('../lib/template-functions'); +const {groupBy} = require('../lib/outputters/templating/template-functions'); exports = module.exports = function({ cliArgs = {}, From f304e28d93e964524626299c479fa640aeda5817 Mon Sep 17 00:00:00 2001 From: Fabio Beltramini Date: Tue, 11 Jul 2023 18:59:03 -0400 Subject: [PATCH 02/10] Migrated markdownDeveloper output mode to outputters directory --- index.js | 16 +--------------- lib/outputters/index.js | 3 ++- lib/outputters/markdown-developer.js | 24 ++++++++++++++++++++++++ lib/outputters/markdown.js | 2 +- 4 files changed, 28 insertions(+), 17 deletions(-) create mode 100644 lib/outputters/markdown-developer.js diff --git a/index.js b/index.js index f1e7927..6bd366a 100755 --- a/index.js +++ b/index.js @@ -237,7 +237,7 @@ module.exports = async function( break; } case 'markdown-developer': - await outputDeveloperMarkdown(messages); + await outputters.markdownDeveloper(messages, {console}); break; case 'jenkins': await outputJenkins(messages); @@ -309,20 +309,6 @@ module.exports = async function( fs.writeFileSync('results.json', json, 'utf8'); } - /** - * Output developer.md, which may help developers navigate the project. Available for backwards compatibility, but generally less used. - * - * @param {array} messages Array of messages - * @return {void} - */ - async function outputDeveloperMarkdown(messages) { - console.log('Writing developer.md files...'); - const asyncTemplates = require('./lib/templates.js'); - const templates = await asyncTemplates; - fs.writeFileSync('developer.md', templates.developer({messages}).replace(/\n\t+/g, '\n')); - console.log('> developer.md done'); - } - /** * Output errors to the command line in a legacy format. Available for backwards compatibility, but 'lines' is generally better. * diff --git a/lib/outputters/index.js b/lib/outputters/index.js index 55a4f27..cef5997 100644 --- a/lib/outputters/index.js +++ b/lib/outputters/index.js @@ -1,5 +1,6 @@ const outputters = { - markdown: require("./markdown.js") + markdown: require("./markdown.js"), + markdownDeveloper: require("./markdown-developer.js") }; module.exports = outputters; \ No newline at end of file diff --git a/lib/outputters/markdown-developer.js b/lib/outputters/markdown-developer.js new file mode 100644 index 0000000..80e6b13 --- /dev/null +++ b/lib/outputters/markdown-developer.js @@ -0,0 +1,24 @@ +const defaultConsole = console; +const defaultFs = require('node:fs/promises'); +const markdown = require('./markdown.js'); + + + /** + * Output developer.md, which may help developers navigate the project. Available for backwards compatibility, but generally less used. + * + * @param {array} messages Array of messages + * @param {object} options.console I/O override for console + * @param {object} options.fs I/O override for fs (filesystem) + * @return {void} + */ + async function markdownDeveloper(messages, { + console = defaultConsole, + fs = defaultFs + }) { + console.log('Writing developer.md files...'); + const templates = await require('./templating/templates.js'); + await fs.writeFile('developer.md', templates.developer({messages}).replace(/\n\t+/g, '\n')); + console.log('> developer.md done'); + } + +module.exports = markdownDeveloper diff --git a/lib/outputters/markdown.js b/lib/outputters/markdown.js index 0c9d6bd..ec76eb6 100644 --- a/lib/outputters/markdown.js +++ b/lib/outputters/markdown.js @@ -16,7 +16,7 @@ async function markdown(messages, { dateOutput, console = defaultConsole, fs = defaultFs - }) { + }={}) { console.log('Writing issues.md...'); const asyncTemplates = require('./templating/templates.js'); const templates = await asyncTemplates; From 317ac37b20b3473e6376bf0975e96c913f5f2711 Mon Sep 17 00:00:00 2001 From: Fabio Beltramini Date: Fri, 14 Jul 2023 16:07:24 -0400 Subject: [PATCH 03/10] Migrated jenkins output to outputters directory --- index.js | 23 +---------------------- lib/outputters/index.js | 3 ++- lib/outputters/jenkins.js | 29 +++++++++++++++++++++++++++++ 3 files changed, 32 insertions(+), 23 deletions(-) create mode 100644 lib/outputters/jenkins.js diff --git a/index.js b/index.js index 6bd366a..fa10f13 100755 --- a/index.js +++ b/index.js @@ -240,7 +240,7 @@ module.exports = async function( await outputters.markdownDeveloper(messages, {console}); break; case 'jenkins': - await outputJenkins(messages); + await outputters.jenkins(messages, {console}); break; case 'lines': { const verbose = options.verbose || false; @@ -288,27 +288,6 @@ module.exports = async function( return; - /** - * Output results.json for sample Jenkins configuration. To be used with markdown as well by default. - * - * @param {array} messages Array of messages - * @return {void} - */ - async function outputJenkins(messages) { - let errors = messages.filter((msg) => { - return msg.level === 'error'; - }); - const buildStatus = errors.length ? 'FAILED' : 'PASSED'; - console.log(`BUILD ${buildStatus}: ${errors.length} errors found. Check .md files for details.`); - let json = JSON.stringify({ - buildStatus: buildStatus, - errors: errors.length, - warnings: 0, - lamsErrors: 0, - }); - fs.writeFileSync('results.json', json, 'utf8'); - } - /** * Output errors to the command line in a legacy format. Available for backwards compatibility, but 'lines' is generally better. * diff --git a/lib/outputters/index.js b/lib/outputters/index.js index cef5997..9d496cd 100644 --- a/lib/outputters/index.js +++ b/lib/outputters/index.js @@ -1,6 +1,7 @@ const outputters = { markdown: require("./markdown.js"), - markdownDeveloper: require("./markdown-developer.js") + markdownDeveloper: require("./markdown-developer.js"), + jenkins: require("./jenkins.js"), }; module.exports = outputters; \ No newline at end of file diff --git a/lib/outputters/jenkins.js b/lib/outputters/jenkins.js new file mode 100644 index 0000000..9de5ecc --- /dev/null +++ b/lib/outputters/jenkins.js @@ -0,0 +1,29 @@ +const defaultConsole = console; +const defaultFs = require('node:fs/promises'); + +/** + * Output results.json for sample Jenkins configuration. To be used with markdown as well by default. + * + * @param {array} messages Array of messages + * @return {void} + */ +async function jenkins(messages, { + dateOutput, + console = defaultConsole, + fs = defaultFs + }={}) { + let errors = messages.filter((msg) => { + return msg.level === 'error'; + }); + const buildStatus = errors.length ? 'FAILED' : 'PASSED'; + console.log(`BUILD ${buildStatus}: ${errors.length} errors found. Check .md files for details.`); + let json = JSON.stringify({ + buildStatus: buildStatus, + errors: errors.length, + warnings: 0, + lamsErrors: 0, + }); + await fs.writeFile('results.json', json, 'utf8'); +} + +module.exports = jenkins \ No newline at end of file From c0066e60727570f8fab3b260a2481c3188cc4aa0 Mon Sep 17 00:00:00 2001 From: Fabio Beltramini Date: Fri, 14 Jul 2023 16:38:18 -0400 Subject: [PATCH 04/10] Migrated legacy-cli output to outputters directory --- index.js | 19 +------------------ lib/outputters/index.js | 1 + lib/outputters/legacy-cli.js | 20 ++++++++++++++++++++ 3 files changed, 22 insertions(+), 18 deletions(-) create mode 100644 lib/outputters/legacy-cli.js diff --git a/index.js b/index.js index fa10f13..c2ef58d 100755 --- a/index.js +++ b/index.js @@ -248,7 +248,7 @@ module.exports = async function( break; } case 'legacy-cli': - await outputLegacyCli(messages); + await outputters.legacyCli(messages); break; default: console.warn(`Unrecognized output mode: ${output}`); @@ -288,23 +288,6 @@ module.exports = async function( return; - /** - * Output errors to the command line in a legacy format. Available for backwards compatibility, but 'lines' is generally better. - * - * @param {array} messages Array of messages - * @return {void} - */ - function outputLegacyCli(messages) { - let maxArrayLength = 5000; - let errors = messages.filter((msg) => { - return msg.level === 'error'; - }); - if (errors.length) { - console.log('Errors:'); - console.dir(errors, {maxArrayLength}); - } - } - /** * Output info and errors to the command line in a human-readable line-by-line format. * diff --git a/lib/outputters/index.js b/lib/outputters/index.js index 9d496cd..0944bd4 100644 --- a/lib/outputters/index.js +++ b/lib/outputters/index.js @@ -2,6 +2,7 @@ const outputters = { markdown: require("./markdown.js"), markdownDeveloper: require("./markdown-developer.js"), jenkins: require("./jenkins.js"), + legacyCli: require("./legacy-cli.js") }; module.exports = outputters; \ No newline at end of file diff --git a/lib/outputters/legacy-cli.js b/lib/outputters/legacy-cli.js new file mode 100644 index 0000000..9c0bfa3 --- /dev/null +++ b/lib/outputters/legacy-cli.js @@ -0,0 +1,20 @@ +const defaultConsole = console; + +/** + * Output errors to the command line in a legacy format. Available for backwards compatibility, but 'lines' is generally better. + * + * @param {array} messages Array of messages + * @return {void} + */ +function legacyCli(messages, {console=defaultConsole}={}) { + let maxArrayLength = 5000; + let errors = messages.filter((msg) => { + return msg.level === 'error'; + }); + if (errors.length) { + console.log('Errors:'); + console.dir(errors, {maxArrayLength}); + } +} + +module.exports = legacyCli \ No newline at end of file From 607757e7c70e40f840a2b5efc64d14e2996fe784 Mon Sep 17 00:00:00 2001 From: Fabio Beltramini Date: Fri, 14 Jul 2023 16:50:58 -0400 Subject: [PATCH 05/10] Migrated lines output to outputters directory --- index.js | 52 +--------------------------------------- lib/outputters/index.js | 7 +++--- lib/outputters/lines.js | 53 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 58 insertions(+), 54 deletions(-) create mode 100644 lib/outputters/lines.js diff --git a/index.js b/index.js index c2ef58d..54a22ae 100755 --- a/index.js +++ b/index.js @@ -239,12 +239,9 @@ module.exports = async function( case 'markdown-developer': await outputters.markdownDeveloper(messages, {console}); break; - case 'jenkins': - await outputters.jenkins(messages, {console}); - break; case 'lines': { const verbose = options.verbose || false; - await outputLines(messages, {verbose}); + await outputters.lines(messages, {verbose, console}); break; } case 'legacy-cli': @@ -287,51 +284,4 @@ module.exports = async function( } return; - - /** - * Output info and errors to the command line in a human-readable line-by-line format. - * - * @param {array} messages Array of messages - * @param {object} options Options - * @param {boolean} options.verbose Whether to output verbose-level messages - * @return {void} - */ - async function outputLines(messages, {verbose}) { - const cols = { - level: {header: '', width: 3}, - rule: {header: 'Rule', width: 7}, - location: {header: 'Location', width: 47}, - description: {header: 'Description'}, - }; - const levels = { - info: {sorting: 1, icon: '🛈'}, - verbose: {sorting: 1, icon: '💬'}, - error: {sorting: 2, icon: '❌'}, - }; - let sortedMessages = messages.sort((a, b)=>{ - let aLevel = levels[a.level] || {sorting: 0}; - let bLevel = levels[b.level] || {sorting: 0}; - return (aLevel.sorting - bLevel.sorting) - || (a.rule||'').localeCompare(b.rule||''); - }); - console.log(Object.values(cols).map((col)=>col.header.padEnd(col.width||0)).join('\t')); - for (let message of sortedMessages) { - if (message.level === 'verbose' && !verbose) { - continue; - } - /* eslint indent: ["error", 4, { "flatTernaryExpressions": true }]*/ - const level = (levels[message.level].icon || message.level.toString()) - .slice(0, cols.level.width) - .padEnd(cols.level.width, ' '); - const rule = (message.rule || '') - .slice(0, cols.rule.width) - .padEnd(cols.rule.width, '.'); - const location = (message.location || '') - .replace(/model:|view:|explore:|join:|dimension:|measure:|filter:/g, (match)=>match.slice(0, 1)+':') - .slice(-cols.location.width) - .padEnd(cols.location.width, '.'); - const description = message.description || ''; - console.log([level, rule, location, description].join('\t')); - } - } }; diff --git a/lib/outputters/index.js b/lib/outputters/index.js index 0944bd4..f3bd353 100644 --- a/lib/outputters/index.js +++ b/lib/outputters/index.js @@ -1,8 +1,9 @@ const outputters = { + jenkins: require("./jenkins.js"), + legacyCli: require("./legacy-cli.js"), + lines: require("./lines.js"), markdown: require("./markdown.js"), markdownDeveloper: require("./markdown-developer.js"), - jenkins: require("./jenkins.js"), - legacyCli: require("./legacy-cli.js") }; -module.exports = outputters; \ No newline at end of file +module.exports = outputters; diff --git a/lib/outputters/lines.js b/lib/outputters/lines.js new file mode 100644 index 0000000..e253ba2 --- /dev/null +++ b/lib/outputters/lines.js @@ -0,0 +1,53 @@ +const defaultConsole = console; + +/** + * Output info and errors to the command line in a human-readable line-by-line format. + * + * @param {array} messages Array of messages + * @param {object} options Options + * @param {boolean} options.verbose Whether to output verbose-level messages + * @return {void} + */ +async function lines(messages, { + verbose, + console=defaultConsole + }={}) { + const cols = { + level: {header: '', width: 3}, + rule: {header: 'Rule', width: 7}, + location: {header: 'Location', width: 47}, + description: {header: 'Description'}, + }; + const levels = { + info: {sorting: 1, icon: '🛈'}, + verbose: {sorting: 1, icon: '💬'}, + error: {sorting: 2, icon: '❌'}, + }; + let sortedMessages = messages.sort((a, b)=>{ + let aLevel = levels[a.level] || {sorting: 0}; + let bLevel = levels[b.level] || {sorting: 0}; + return (aLevel.sorting - bLevel.sorting) + || (a.rule||'').localeCompare(b.rule||''); + }); + console.log(Object.values(cols).map((col)=>col.header.padEnd(col.width||0)).join('\t')); + for (let message of sortedMessages) { + if (message.level === 'verbose' && !verbose) { + continue; + } + /* eslint indent: ["error", 4, { "flatTernaryExpressions": true }]*/ + const level = (levels[message.level].icon || message.level.toString()) + .slice(0, cols.level.width) + .padEnd(cols.level.width, ' '); + const rule = (message.rule || '') + .slice(0, cols.rule.width) + .padEnd(cols.rule.width, '.'); + const location = (message.location || '') + .replace(/model:|view:|explore:|join:|dimension:|measure:|filter:/g, (match)=>match.slice(0, 1)+':') + .slice(-cols.location.width) + .padEnd(cols.location.width, '.'); + const description = message.description || ''; + console.log([level, rule, location, description].join('\t')); + } +} + +module.exports = lines; From 5121b1ebf207e8762723af57a9db691133ac6422 Mon Sep 17 00:00:00 2001 From: Fabio Beltramini Date: Sat, 29 Jul 2023 20:08:16 -0400 Subject: [PATCH 06/10] Added initial outputter implementation, added failing test (need to add feature for central exemptions) --- .../27-output-add-exemptions/.gitignore | 1 + .../27-output-add-exemptions/index.test.js | 93 +++++++++++++++++++ .../27-output-add-exemptions/manifest.lkml | 2 + .../27-output-add-exemptions/test.model.lkml | 7 ++ index.js | 5 + lib/outputters/add-exemptions.js | 60 ++++++++++++ lib/outputters/index.js | 1 + 7 files changed, 169 insertions(+) create mode 100644 __tests__/dummy-projects/27-output-add-exemptions/.gitignore create mode 100644 __tests__/dummy-projects/27-output-add-exemptions/index.test.js create mode 100644 __tests__/dummy-projects/27-output-add-exemptions/manifest.lkml create mode 100644 __tests__/dummy-projects/27-output-add-exemptions/test.model.lkml create mode 100644 lib/outputters/add-exemptions.js diff --git a/__tests__/dummy-projects/27-output-add-exemptions/.gitignore b/__tests__/dummy-projects/27-output-add-exemptions/.gitignore new file mode 100644 index 0000000..9f5cea4 --- /dev/null +++ b/__tests__/dummy-projects/27-output-add-exemptions/.gitignore @@ -0,0 +1 @@ +lams-exemptions.ndjson diff --git a/__tests__/dummy-projects/27-output-add-exemptions/index.test.js b/__tests__/dummy-projects/27-output-add-exemptions/index.test.js new file mode 100644 index 0000000..f76be33 --- /dev/null +++ b/__tests__/dummy-projects/27-output-add-exemptions/index.test.js @@ -0,0 +1,93 @@ +const lams = require('../../../index.js') +const mocks = require('../../../lib/mocks.js') +const path= require('path') +const defaultTestingOptions = {reporting:"no", cwd:__dirname} +require('../../../lib/expect-to-contain-message'); +const log = x=>console.log(x) +const testProjectName = __dirname.split(path.sep).slice(-1)[0]; +const fs = require("node:fs/promises") +const expectedOutput = ` +{"rule":"K1","location":"model:test/view:bad"} +`; +describe('Projects', () => { + describe(testProjectName, () => { + let {spies, process, console} = mocks() + let messages1, messages2, output1, output2 + beforeAll( async () => { + const options = { + ...defaultTestingOptions, + output: "add-exemptions" + } + const outputPath = path.resolve(__dirname,"lams-exemptions.ndjson") + log(outputPath) + try{await fs.rm(outputPath, {force:true})}catch(e){} + messages1 = {messages: await lams(options,{process, console})} + output1 = await fs.readFile(outputPath,{encoding:"utf8"}) + messages2 = {messages: await lams(options,{process, console})} + output2 = await fs.readFile(outputPath,{encoding:"utf8"}) + }) + it("should not error out", ()=> { + expect(console.error).not.toHaveBeenCalled() + }); + it("it should not contain any unexpected parser (P0) errors", ()=> { + expect(messages1).not.toContainMessage({ + rule: "P0", + level: "error" + }); + expect(messages2).not.toContainMessage({ + rule: "P0", + level: "error" + }); + }); + it("it should not contain any parser syntax (P1) errors", ()=> { + expect(messages1).not.toContainMessage({ + rule: "P1", + level: "error" + }); + expect(messages2).not.toContainMessage({ + rule: "P1", + level: "error" + }); + }); + + it("run 1 should match K1 (1 match, 0 exempt, 1 error)", ()=> { + expect(messages1).toContainMessage({ + rule: "K1", + level: "info", + description: "Rule K1 summary: 1 matches, 0 matches exempt, and 1 errors" + }); + }); + + it("run 1 should error on K1", ()=> { + expect(messages1).toContainMessage({ + rule: "K1", + level: "error" + }); + }); + + it("run 1 should output the expected ndjson", ()=> { + expect(output1).toEqual(expectedOutput); + }); + + it("run 2 should match K1 (1 match, 1 exempt, 0 errors)", ()=> { + expect(messages2).toContainMessage({ + rule: "K1", + level: "info", + description: "Rule K1 summary: 1 matches, 1 matches exempt, and 0 errors" + }); + }); + + it("run 2 not should error on K1", ()=> { + expect(messages2).not.toContainMessage({ + rule: "K1", + level: "error" + }); + }); + + it("run 2 should output the (same) expected ndjson", ()=> { + expect(output2).toEqual(expectedOutput); + }); + + + }); +}); diff --git a/__tests__/dummy-projects/27-output-add-exemptions/manifest.lkml b/__tests__/dummy-projects/27-output-add-exemptions/manifest.lkml new file mode 100644 index 0000000..44e373c --- /dev/null +++ b/__tests__/dummy-projects/27-output-add-exemptions/manifest.lkml @@ -0,0 +1,2 @@ +#LAMS +#rule: K1 {} diff --git a/__tests__/dummy-projects/27-output-add-exemptions/test.model.lkml b/__tests__/dummy-projects/27-output-add-exemptions/test.model.lkml new file mode 100644 index 0000000..0b03bf7 --- /dev/null +++ b/__tests__/dummy-projects/27-output-add-exemptions/test.model.lkml @@ -0,0 +1,7 @@ + +explore: bad {} + +view: bad { + sql_table_name: foo ;; + dimension: not_a_pk {} +} \ No newline at end of file diff --git a/index.js b/index.js index 54a22ae..225e6e3 100755 --- a/index.js +++ b/index.js @@ -231,6 +231,11 @@ module.exports = async function( for (let output of outputModes.split(',')) { switch (output) { case '': break; + case 'add-exemptions':{ + const verbose = options.verbose || false; + await outputters.addExemptions(messages, {verbose, cwd, console}) + break; + } case 'markdown': { const {dateOutput} = options; await outputters.markdown(messages, {dateOutput,console}); diff --git a/lib/outputters/add-exemptions.js b/lib/outputters/add-exemptions.js new file mode 100644 index 0000000..fce7b97 --- /dev/null +++ b/lib/outputters/add-exemptions.js @@ -0,0 +1,60 @@ +const defaultConsole = console; +const defaultFs = require('node:fs/promises'); +const readline = require('node:readline/promises'); +const pathLib = require('path'); + +/** + * Read a prior state from a file, output a new state to that file, and return the incremental messages + * + * @param {array} messages Array of messages + * @param {object} options Options + * @param {boolean} options.verbose Whether to output verbose-level messages + * @return {void} + */ +async function addExemptions(messages, { + verbose = false, + cwd = process.cwd(), + incrementalPath, + console = defaultConsole, + fs = defaultFs + }={}) { + + if(!messages.some(m => m.level==="error")){ + //Short circuit cases where no new records would be written + return + } + + const centralExemptionsPath = pathLib.resolve(cwd, incrementalPath || "lams-exemptions.ndjson") + + // Append error messages into lams-exemptions.ndjson + let file + try{ + file = await fs.open(centralExemptionsPath,"a") + + // Add an additional line break to the start of the batch + // This can defend against someone accidentally saving the file without a terminating line break + // And it can also help users identify batches from distinct runs + // Per the ndjson spec, and per our own implementation, any empty lines will simply be ignored + await file.write("\n") + + for(let message of messages){ + if(message.level !== "error"){ + //Only need to add exemptions for errors + continue + } + await file.write( + JSON.stringify({ + rule: message.rule, + location: message.location, + }) + +"\n" + ) + } + } + catch(e){ + console.log(e) + } + await file?.close() +} + +module.exports = addExemptions; diff --git a/lib/outputters/index.js b/lib/outputters/index.js index f3bd353..b716980 100644 --- a/lib/outputters/index.js +++ b/lib/outputters/index.js @@ -1,4 +1,5 @@ const outputters = { + addExemptions: require('./add-exemptions'), jenkins: require("./jenkins.js"), legacyCli: require("./legacy-cli.js"), lines: require("./lines.js"), From c2de372fbe76423ae35021d2c9aab6cff935e43f Mon Sep 17 00:00:00 2001 From: Fabio Beltramini Date: Wed, 2 Aug 2023 19:19:15 -0400 Subject: [PATCH 07/10] Incremental (output=add-exemptions) first draft. Adds project.centralExemptions, loads it from lams-exemptions.ndjson, and allows outputting errors back to this file to exempt them on future runs --- .../27-output-add-exemptions/index.test.js | 7 +- index.js | 16 +++- lib/custom-rule/format-location.js | 12 +++ lib/custom-rule/result-interpreter.js | 15 +--- lib/get-exemption-deep.js | 28 +++++-- lib/loaders/lams-exemptions.js | 74 +++++++++++++++++++ lib/outputters/add-exemptions.js | 11 +-- 7 files changed, 129 insertions(+), 34 deletions(-) create mode 100644 lib/custom-rule/format-location.js create mode 100644 lib/loaders/lams-exemptions.js diff --git a/__tests__/dummy-projects/27-output-add-exemptions/index.test.js b/__tests__/dummy-projects/27-output-add-exemptions/index.test.js index f76be33..3d0c6c1 100644 --- a/__tests__/dummy-projects/27-output-add-exemptions/index.test.js +++ b/__tests__/dummy-projects/27-output-add-exemptions/index.test.js @@ -6,9 +6,7 @@ require('../../../lib/expect-to-contain-message'); const log = x=>console.log(x) const testProjectName = __dirname.split(path.sep).slice(-1)[0]; const fs = require("node:fs/promises") -const expectedOutput = ` -{"rule":"K1","location":"model:test/view:bad"} -`; +const expectedOutput = '{"rule":"K1","location":"model:test/view:bad"}\n'; describe('Projects', () => { describe(testProjectName, () => { let {spies, process, console} = mocks() @@ -18,8 +16,7 @@ describe('Projects', () => { ...defaultTestingOptions, output: "add-exemptions" } - const outputPath = path.resolve(__dirname,"lams-exemptions.ndjson") - log(outputPath) + const outputPath = path.resolve(__dirname,"lams-exemptions.ndjson") try{await fs.rm(outputPath, {force:true})}catch(e){} messages1 = {messages: await lams(options,{process, console})} output1 = await fs.readFile(outputPath,{encoding:"utf8"}) diff --git a/index.js b/index.js index 225e6e3..4fe1398 100755 --- a/index.js +++ b/index.js @@ -103,6 +103,7 @@ module.exports = async function( } console.log('> Parsing done!'); + console.log('Getting manifest and exemption info...'); // Loading project manifest settings const manifestInfo = {level: 'info', location: 'project'}; if (project.manifest) { @@ -124,12 +125,23 @@ module.exports = async function( messages.push({...manifestInfo, description: `Rules: ${ruleKeys.slice(0, 6).join(', ')}${ruleKeys.length>6?'...':''}`}); } + //Loading central exemptions + { + const {lamsRuleExemptionsPath} = options + const loadLamsExemptions = require('./lib/loaders/lams-exemptions.js') + const result = await loadLamsExemptions({cwd, lamsRuleExemptionsPath}) + messages = messages.concat(result.messages || []) + project.centralExemptions = result.centralExemptions + } + project.name = false || project.manifest && project.manifest.project_name || options.projectName || (options.cwd || process.cwd() || '').split(path.sep).filter(Boolean).slice(-1)[0] // The current directory. May not actually be the project name... || 'unknown_project'; + console.log('> Manifest and exemptions done'); + console.log('Checking rules... '); let builtInRuleNames = @@ -232,8 +244,8 @@ module.exports = async function( switch (output) { case '': break; case 'add-exemptions':{ - const verbose = options.verbose || false; - await outputters.addExemptions(messages, {verbose, cwd, console}) + const lamsRuleExemptionsPath = options.lamsRuleExemptionsPath; + await outputters.addExemptions(messages, {cwd, console, lamsRuleExemptionsPath}) break; } case 'markdown': { diff --git a/lib/custom-rule/format-location.js b/lib/custom-rule/format-location.js new file mode 100644 index 0000000..a23a956 --- /dev/null +++ b/lib/custom-rule/format-location.js @@ -0,0 +1,12 @@ +/** + * Given an array representing a path to a json location, formats it as a more readable LookML-specific "location" string. + * @param {Array} pathArray Path array to format. + * @return {string} The formatted location. + */ + +module.exports = function formatLocation(pathArray) { + return pathArray?.join('/').replace( + /(^|\/)(model|file|view|join|explore|datagroup|dimension|measure|filter|parameter)\//g, + (match) => match.slice(0, -1) + ':', + ); +} \ No newline at end of file diff --git a/lib/custom-rule/result-interpreter.js b/lib/custom-rule/result-interpreter.js index 9cef3e3..62f34a4 100644 --- a/lib/custom-rule/result-interpreter.js +++ b/lib/custom-rule/result-interpreter.js @@ -1,5 +1,6 @@ const getExemptionDeep = require('../get-exemption-deep.js'); +const formatLocation = require('./format-location.js'); const CodeError = require('../code-error.js'); const ifValidLevel = (l) => ["verbose","info","error"].includes(l) && l @@ -14,7 +15,7 @@ module.exports = function resultInterpreter(ruleDef={}){ const {match,value} = result const resultDefault = { ...ruleDefault, - location: formatPath(match?.path) + location: formatLocation(match?.path) } const resultRule = // E.g., result may specify a sub-rule, e.g. T2.3 value?.rule @@ -65,18 +66,6 @@ module.exports = function resultInterpreter(ruleDef={}){ } } -/** - * Throw custom error. - * @param {Array} pathArray Path array to format. - * @return {string} The formatted path. - */ -function formatPath(pathArray) { - return pathArray?.join('/').replace( - /(^|\/)(model|file|view|join|explore|datagroup|dimension|measure|filter|parameter)\//g, - (match) => match.slice(0, -1) + ':', - ); -} - function stringify(thing,max=40){ if(thing===undefined){return "undefined"} let str diff --git a/lib/get-exemption-deep.js b/lib/get-exemption-deep.js index 51dbe3a..471e218 100644 --- a/lib/get-exemption-deep.js +++ b/lib/get-exemption-deep.js @@ -1,12 +1,30 @@ const getExemption = require('./get-exemption.js'); +const formatLocation = require('./custom-rule/format-location.js') module.exports = function getExemptionDeep({match, rule}) { // Check match for being exempt and return early (without evaluating expression) if so const {path, project} = match; + + // Check the manifest for global exemptions const manifestExempt = getExemption(project && project.manifest, rule); - const exempt = - manifestExempt - || path.reduce( + if(manifestExempt){ + return {match, exempt: manifestExempt} + } + + // Check for central exemptions + if(project.centralExemptions){ + for(let p=0, P=path.length;p({ exempt: exempt || getExemption(modelFragment[pathpart], rule), modelFragment: modelFragment[pathpart], @@ -14,10 +32,10 @@ module.exports = function getExemptionDeep({match, rule}) { {modelFragment: project}, ).exempt; - if (exempt) { + if (localExempt) { return { match, - exempt, + exempt: localExempt, }; } }; diff --git a/lib/loaders/lams-exemptions.js b/lib/loaders/lams-exemptions.js new file mode 100644 index 0000000..ff2e613 --- /dev/null +++ b/lib/loaders/lams-exemptions.js @@ -0,0 +1,74 @@ +const defaultFs = require("node:fs") +const readline = require('node:readline/promises'); +const pathLib = require("path") +const defaultLamsRuleExemptionsPath = "lams-exemptions.ndjson" +const location = "file:lams-exemptions.ndjson" + +module.exports = async function loadLamsExcemptions({ + fs = defaultFs, + cwd = process.cwd(), + lamsRuleExemptionsPath = defaultLamsRuleExemptionsPath +}){ + const messages = [] + const centralExemptions = new Set() + try { + const resolvedLamsRuleExemptionsPath = pathLib.resolve(cwd, lamsRuleExemptionsPath) + const fileStream = fs.createReadStream(resolvedLamsRuleExemptionsPath) + const lines = readline.createInterface({ + input: fileStream, + crlfDelay: Infinity, + }) + + let l=0 + for await (const line of lines) { + l++; + if(line === ""){continue} + const data = tryJsonParse(line) + const dataInvalid = validateData(data) + if(dataInvalid){ + messages.push({level:"error",location,description:`${dataInvalid} (line ${l})`}) + continue + } + centralExemptions.add( + data.rule + " "+ data.location + ) + } + + return { + messages, + centralExemptions + } + } + catch(e){ + if(e.code === "ENOENT"){ + messages.push({ + location, + level: lamsRuleExemptionsPath === defaultLamsRuleExemptionsPath ? "verbose" : "error", + description: `No central LAMS exemptions found. ${e.message}` + }) + } + else{ + messages.push({ + location, level: "error", + description: `Unexpected error loading central LAMS exemptions. ${e.message}` + }) + } + return { + messages, + centralExemptions + } + } +} + +function validateData(data){ + if(!data){return `Invalid JSON`} + if(!data.rule){return 'Missing required `rule`'} + if(!data.location){return 'Missing required `location`'} + if(data.rule.match(" ")){return 'Rule name contains an illegal space'} + if(data.location.match(" ")){return 'Location contains an illegal space'} +} + +function tryJsonParse(str,dft){ + try{return JSON.parse(str)} + catch(e){return dft} +} \ No newline at end of file diff --git a/lib/outputters/add-exemptions.js b/lib/outputters/add-exemptions.js index fce7b97..23a21b2 100644 --- a/lib/outputters/add-exemptions.js +++ b/lib/outputters/add-exemptions.js @@ -12,9 +12,8 @@ const pathLib = require('path'); * @return {void} */ async function addExemptions(messages, { - verbose = false, cwd = process.cwd(), - incrementalPath, + lamsRuleExemptionsPath = "lams-exemptions.ndjson", console = defaultConsole, fs = defaultFs }={}) { @@ -24,18 +23,12 @@ async function addExemptions(messages, { return } - const centralExemptionsPath = pathLib.resolve(cwd, incrementalPath || "lams-exemptions.ndjson") + const centralExemptionsPath = pathLib.resolve(cwd, lamsRuleExemptionsPath) // Append error messages into lams-exemptions.ndjson let file try{ file = await fs.open(centralExemptionsPath,"a") - - // Add an additional line break to the start of the batch - // This can defend against someone accidentally saving the file without a terminating line break - // And it can also help users identify batches from distinct runs - // Per the ndjson spec, and per our own implementation, any empty lines will simply be ignored - await file.write("\n") for(let message of messages){ if(message.level !== "error"){ From e374cdd6af4a72aea2cd56a681ad65196f51c423 Mon Sep 17 00:00:00 2001 From: Fabio Beltramini Date: Thu, 3 Aug 2023 20:44:54 -0400 Subject: [PATCH 08/10] Logging and docs --- README.md | 18 ++++++++++++++---- docs/github-action.md | 18 +++++++++++------- docs/release-notes/v3.md | 10 ++++++++++ lib/get-exemption-deep.js | 1 - lib/loaders/lams-exemptions.js | 4 ++++ lib/outputters/add-exemptions.js | 2 ++ 6 files changed, 41 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index a22b475..f168372 100644 --- a/README.md +++ b/README.md @@ -49,9 +49,13 @@ As of LAMS v3, you must opt-in via your `manifest.lkml` file to use the built-in ``` +### Custom Rules + +In addition to linting against its [style guide](https://looker-open-source.github.io/look-at-me-sideways/rules.html), LAMS also lets you specify your own rules. See [Customizing LAMS](https://looker-open-source.github.io/look-at-me-sideways/customizing-lams). + ### Rule Exemptions -You can opt-out of rules granularly using `rule_exemptions`. +You can opt-out of rules granularly by locally specifying `rule_exemptions`. The rule exemption syntax encourages developers to document the reason for each such exemption: @@ -67,9 +71,15 @@ view: rollup { ... ``` -### Custom Rules +(BETA) You can also opt-out of rules granularly from a centrally maintained `lams-exemptions.ndjson` file. Simply specify the rule name and location to exempt in a series of newline-terminated JSON objects: -In addition to linting against its [style guide](https://looker-open-source.github.io/look-at-me-sideways/rules.html), LAMS also lets you specify your own rules. See [Customizing LAMS](https://looker-open-source.github.io/look-at-me-sideways/customizing-lams). +```js +{"rule":"K3","location":"model:my_model/view:rollup"} +{"rule":"K3","location":"model:my_other_model/view:foo"} + +``` + +You may also apply rule_exemptions globally in your project.manifest, but this is generally unnecessary as of LAMS v3. ### Output @@ -112,7 +122,7 @@ The following examples were prepared for v1 of LAMS, though updating them for v2 - **reporting** - Required. One of `yes`, `no`, `save-yes`, or `save-no`. See [PRIVACY.md](https://github.com/looker-open-source/look-at-me-sideways/blob/master/PRIVACY.md) for details. - **report-user** - An email address to use in reporting. See [PRIVACY.md](https://github.com/looker-open-source/look-at-me-sideways/blob/master/PRIVACY.md) for details. - **report-license-key** - A Looker license key to use in reporting. See [PRIVACY.md](https://github.com/looker-open-source/look-at-me-sideways/blob/master/PRIVACY.md) for details. -- **output** - A comma-separated string of output modes from among: `lines` (default), `markdown`, `markdown-developer`, `jenkins`, `legacy-cli` +- **output** - A comma-separated string of output modes from among: `lines` (default), `markdown`, `markdown-developer`, `jenkins`, `legacy-cli`, or (BETA) `add-exemptions` - **source** - A glob specifying which files to read. Defaults to `**/{*.model,*.explore,*.view,manifest}.lkml`. - **cwd** - A path for LAMS to use as its current working directory. Useful if you are not invoking lams from your LookML repo directory. - **project-name** - An optional name for the project, used to generate links back to the project in mardown output. Specifying this in manifest.lkml is preferred. diff --git a/docs/github-action.md b/docs/github-action.md index d7149fe..710332f 100644 --- a/docs/github-action.md +++ b/docs/github-action.md @@ -3,14 +3,14 @@ favicon: img/logo.png --- # Running LAMS via Github Actions -This example shows how to run LAMS with Github Actions. +- Inside your Github-connected LookML repo, you can specify [Github Actions](https://docs.github.com/en/actions) in `.github/workflows/main.yml` +- Populate the parameters within the "Run LAMS" step (see `XXXXX` placeholders below) +- Note: This configuration is helpful alongside pull requests. [Set "Pull Requests Required" within Looker](https://cloud.google.com/looker/docs/git-options#integrating_pull_requests_for_your_project). +- Note: This configuration is helpful alongside Github's [protected branches](https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repository/managing-protected-branches/about-protected-branches) feature -Note: As compared to our dockerized Jenkins configuration, Github Actions is a quicker way to get set up, but may be slower since the former does not require as much set-up per run (Node.js installation, LAMS installation), and can run on a dedicated instance. +# Standard configuration -## Instructions - -- Inside your Github-connected LookML repo, add the following file at `.github/workflows/main.yml` -- Populate the parameters within the "Run LAMS" step & uncomment +This configuration is useful alongside ```yaml @@ -33,6 +33,10 @@ jobs: run: npm install -g @looker/look-at-me-sideways@3 - name: Run LAMS # See [PRIVACY.md](https://github.com/looker-open-source/look-at-me-sideways/blob/master/PRIVACY.md) - run: lams --reporting=... --report-license-key=... --report-user=... + run: lams --reporting=XXXXX --report-license-key=XXXXX --report-user=XXXXX ``` + +# ([BETA](https://github.com/looker-open-source/look-at-me-sideways/issues/142)) Incremental configuration + +If at any point (for example, manually, or upon merging a set of changes), you wish to exempt all current errors in future runs, you can run LAMS with `--output=add-exemptions` and add the resulting/updated exemptions file to your repo. diff --git a/docs/release-notes/v3.md b/docs/release-notes/v3.md index 1df1f54..e5d19a1 100644 --- a/docs/release-notes/v3.md +++ b/docs/release-notes/v3.md @@ -56,3 +56,13 @@ For any error not detailed below, please submit an issue! - You may wish to update the view to comply with the style guide, or apply exemptions for the relevant rule(s) - If you have opted out of rules T2-T10 - These rules are now all bundled under rule T2. For a global exemption, you may simply omit rule T2 from your manifest. For local exemptions, or exemptions to the subrules, you may still exempt these individually, although the ID's of the rules have been updated. (For example, rule "T3" is now "T2.1") + +# v3.1 + +This update introduces, as beta features, centrally managed exemptions, and an `add-exemptions` output mode. Together, these features serve to enable incremental linting use cases. + +Feedback on these beta features and the overall incremental linting use case is welcome in [issue #142](https://github.com/looker-open-source/look-at-me-sideways/issues/142). + +- **Centrally-managed exemptions**: (BETA) In addition to locally specified exemptions, LAMS can now also use exemptions in `lams-exemptions.ndjson` in a newline-delimited JSON format. + - If you wish to source this file from another location, you can pass the `` argument. +- **Add exemptions as output**: (BETA) `output=add-exemptions` can append any errors from the current run as new exemptions in the `lams-exemptions.ndjson` file. diff --git a/lib/get-exemption-deep.js b/lib/get-exemption-deep.js index 471e218..23d01f5 100644 --- a/lib/get-exemption-deep.js +++ b/lib/get-exemption-deep.js @@ -16,7 +16,6 @@ module.exports = function getExemptionDeep({match, rule}) { for(let p=0, P=path.length;p0){ + console.log("BETA: lams-exemptions.ndjson and incremental linting are in beta. Feedback welcome in LAMS issue #142.") + } + return { messages, centralExemptions diff --git a/lib/outputters/add-exemptions.js b/lib/outputters/add-exemptions.js index 23a21b2..fee5760 100644 --- a/lib/outputters/add-exemptions.js +++ b/lib/outputters/add-exemptions.js @@ -18,6 +18,8 @@ async function addExemptions(messages, { fs = defaultFs }={}) { + console.log("BETA: output=add-exemptions and incremental linting are in beta. Feedback welcome in LAMS issue #142.") + if(!messages.some(m => m.level==="error")){ //Short circuit cases where no new records would be written return From 8af1de361924fc0881a4668ff9fe75c73c5ee245 Mon Sep 17 00:00:00 2001 From: Fabio Beltramini Date: Mon, 7 Aug 2023 19:06:31 -0400 Subject: [PATCH 09/10] Formatting --- docs/github-action.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/github-action.md b/docs/github-action.md index 710332f..86a2225 100644 --- a/docs/github-action.md +++ b/docs/github-action.md @@ -39,4 +39,4 @@ jobs: # ([BETA](https://github.com/looker-open-source/look-at-me-sideways/issues/142)) Incremental configuration -If at any point (for example, manually, or upon merging a set of changes), you wish to exempt all current errors in future runs, you can run LAMS with `--output=add-exemptions` and add the resulting/updated exemptions file to your repo. +If at any point (for example, manually, or upon merging a set of changes), you wish to exempt all current errors in future runs, you can run LAMS with `--output=add-exemptions` and add the resulting/updated `lams-exemptions.ndjson` file to your repo. From 629b209f6e035d3532be8ebff25af1a03bbf39a1 Mon Sep 17 00:00:00 2001 From: Fabio Beltramini Date: Mon, 7 Aug 2023 19:11:12 -0400 Subject: [PATCH 10/10] Lint the linter --- index.js | 488 +++++++++++++++++++------------------- lib/get-exemption-deep.js | 32 +-- 2 files changed, 260 insertions(+), 260 deletions(-) diff --git a/index.js b/index.js index 4fe1398..d9ec369 100755 --- a/index.js +++ b/index.js @@ -34,271 +34,271 @@ const defaultProcess = process; */ module.exports = async function( - options, - { - console = defaultConsole, - process = defaultProcess, - fs, - get, - tracker, - } = {}, + options, + { + console = defaultConsole, + process = defaultProcess, + fs, + get, + tracker, + } = {}, ) { - let messages = []; - try { - fs = fs || require('fs'); - get = get || require('./lib/https-get.js'); - tracker = tracker || require('./lib/tracking')({ - cliArgs: { - reporting: options.reporting, - reportLicenseKey: options.reportLicenseKey, - reportUser: options.reportUser, - }, - gaPropertyId: 'UA-96247573-2', - }); - const path = require('path'); - const parser = require('lookml-parser'); - const checkCustomRule = require('./lib/custom-rule/custom-rule.js'); + let messages = []; + try { + fs = fs || require('fs'); + get = get || require('./lib/https-get.js'); + tracker = tracker || require('./lib/tracking')({ + cliArgs: { + reporting: options.reporting, + reportLicenseKey: options.reportLicenseKey, + reportUser: options.reportUser, + }, + gaPropertyId: 'UA-96247573-2', + }); + const path = require('path'); + const parser = require('lookml-parser'); + const checkCustomRule = require('./lib/custom-rule/custom-rule.js'); - const cwd = options.cwd || process.cwd(); - const ignore = options.ignore || 'node_modules/**'; + const cwd = options.cwd || process.cwd(); + const ignore = options.ignore || 'node_modules/**'; - console.log('Parsing project...'); - const project = await parser.parseFiles({ - source: options.source, - conditionalCommentString: 'LAMS', - fileOutput: 'by-name', - transformations: { - applyExtensionsRefinements: true, - ...(options.transformations||{}), - }, - cwd, - globOptions: { - ignore, - }, - // console: defaultConsole - }); - if (project.error) { // Fatal error - throw (project.error); - } - if (project.errors) { - console.log('> Issues occurred during parsing (containing files will not be considered. See messages for details.)'); - messages = messages.concat(project.errors.map((e) => { - const exception = e && e.error && e.error.exception || {}; - const messageDefault = { - rule: 'P0', - location: `file:${e.$file_path}`, - level: options.onParserError === 'info' ? 'info' : 'error', - description: `Parsing error: ${exception.message || e}`, - path: e.$file_path, - }; - if (exception.name === 'SyntaxError') { - return { - ...messageDefault, - rule: 'P1', - hint: exception.context, - }; - } - return messageDefault; - })); - } - console.log('> Parsing done!'); + console.log('Parsing project...'); + const project = await parser.parseFiles({ + source: options.source, + conditionalCommentString: 'LAMS', + fileOutput: 'by-name', + transformations: { + applyExtensionsRefinements: true, + ...(options.transformations||{}), + }, + cwd, + globOptions: { + ignore, + }, + // console: defaultConsole + }); + if (project.error) { // Fatal error + throw (project.error); + } + if (project.errors) { + console.log('> Issues occurred during parsing (containing files will not be considered. See messages for details.)'); + messages = messages.concat(project.errors.map((e) => { + const exception = e && e.error && e.error.exception || {}; + const messageDefault = { + rule: 'P0', + location: `file:${e.$file_path}`, + level: options.onParserError === 'info' ? 'info' : 'error', + description: `Parsing error: ${exception.message || e}`, + path: e.$file_path, + }; + if (exception.name === 'SyntaxError') { + return { + ...messageDefault, + rule: 'P1', + hint: exception.context, + }; + } + return messageDefault; + })); + } + console.log('> Parsing done!'); - console.log('Getting manifest and exemption info...'); - // Loading project manifest settings - const manifestInfo = {level: 'info', location: 'project'}; - if (project.manifest) { - messages.push({...manifestInfo, description: `Project manifest settings read from ${project.manifest.$file_path}`}); - } else { - messages.push({...manifestInfo, description: `No manifest.lkml file available`}); - } - if (options.manifest) { - messages.push({...manifestInfo, description: `Project manifest settings read from LAMS invocation arguments`}); - } - project.manifest = { - ...(project.manifest||{}), - ...(options.manifest||{}), - }; - const manifestKeys = Object.keys(project.manifest).filter((key)=>key[0]!=='$'); - messages.push({...manifestInfo, description: `Manifest properties: ${manifestKeys.slice(0, 8).join(', ')}${manifestKeys.length>8?'...':''}`}); - if (project.manifest.rule) { - const ruleKeys = Object.keys(project.manifest.rule).filter((key)=>key[0]!=='$'); - messages.push({...manifestInfo, description: `Rules: ${ruleKeys.slice(0, 6).join(', ')}${ruleKeys.length>6?'...':''}`}); - } + console.log('Getting manifest and exemption info...'); + // Loading project manifest settings + const manifestInfo = {level: 'info', location: 'project'}; + if (project.manifest) { + messages.push({...manifestInfo, description: `Project manifest settings read from ${project.manifest.$file_path}`}); + } else { + messages.push({...manifestInfo, description: `No manifest.lkml file available`}); + } + if (options.manifest) { + messages.push({...manifestInfo, description: `Project manifest settings read from LAMS invocation arguments`}); + } + project.manifest = { + ...(project.manifest||{}), + ...(options.manifest||{}), + }; + const manifestKeys = Object.keys(project.manifest).filter((key)=>key[0]!=='$'); + messages.push({...manifestInfo, description: `Manifest properties: ${manifestKeys.slice(0, 8).join(', ')}${manifestKeys.length>8?'...':''}`}); + if (project.manifest.rule) { + const ruleKeys = Object.keys(project.manifest.rule).filter((key)=>key[0]!=='$'); + messages.push({...manifestInfo, description: `Rules: ${ruleKeys.slice(0, 6).join(', ')}${ruleKeys.length>6?'...':''}`}); + } - //Loading central exemptions - { - const {lamsRuleExemptionsPath} = options - const loadLamsExemptions = require('./lib/loaders/lams-exemptions.js') - const result = await loadLamsExemptions({cwd, lamsRuleExemptionsPath}) - messages = messages.concat(result.messages || []) - project.centralExemptions = result.centralExemptions - } + // Loading central exemptions + { + const {lamsRuleExemptionsPath} = options; + const loadLamsExemptions = require('./lib/loaders/lams-exemptions.js'); + const result = await loadLamsExemptions({cwd, lamsRuleExemptionsPath}); + messages = messages.concat(result.messages || []); + project.centralExemptions = result.centralExemptions; + } - project.name = false + project.name = false || project.manifest && project.manifest.project_name || options.projectName || (options.cwd || process.cwd() || '').split(path.sep).filter(Boolean).slice(-1)[0] // The current directory. May not actually be the project name... || 'unknown_project'; - console.log('> Manifest and exemptions done'); + console.log('> Manifest and exemptions done'); - console.log('Checking rules... '); + console.log('Checking rules... '); - let builtInRuleNames = - fs.readdirSync(path.join(__dirname, 'rules')) - .map((fileName) => fileName.match(/^(.*)\.js$/)) // TODO: (v3) rename t2-10.js to just t2.js - .filter(Boolean) - .map((match) => match[1].toUpperCase()); + let builtInRuleNames = + fs.readdirSync(path.join(__dirname, 'rules')) + .map((fileName) => fileName.match(/^(.*)\.js$/)) // TODO: (v3) rename t2-10.js to just t2.js + .filter(Boolean) + .map((match) => match[1].toUpperCase()); - if (!project.manifest) { - console.log(' > No project manifest available in which to find rule declarations/settings.'); - } else if (!project.manifest.rule) { - console.log(' > No rules specified in manifest. As of LAMS v3, built-in rules must be opted-in to.'); - } + if (!project.manifest) { + console.log(' > No project manifest available in which to find rule declarations/settings.'); + } else if (!project.manifest.rule) { + console.log(' > No rules specified in manifest. As of LAMS v3, built-in rules must be opted-in to.'); + } - for (let rule of Object.values(project.manifest && project.manifest.rule || {})) { - console.log('> ' + rule.$name); - if (rule.enabled === false) { - continue; - } - if (builtInRuleNames.includes(rule.$name) && !rule.custom) { // Built-in rule - if (rule.match || rule.expr_rule || rule.description) { - messages.push({ - rule: 'LAMS3', - level: 'info', - description: `Rule ${rule.$name} is a built-in rule. Some rule declaration properties (e.g. match, rule_expr) have been ignored.`, - }); - } - try { - let ruleModule = require('./rules/' + rule.$name.toLowerCase() + '.js'); - let result = ruleModule(project); - messages = messages.concat(result.messages); - } catch (e) { - messages.push({ - rule: 'LAMS1', - level: 'error', - description: `LAMS error evaluating rule ${rule.$name.toUpperCase()}: ${e.message || e}`, - }); - } - } else { - messages = messages.concat(checkCustomRule(rule, project, {console})); - } - } - console.log('> Rules done!'); + for (let rule of Object.values(project.manifest && project.manifest.rule || {})) { + console.log('> ' + rule.$name); + if (rule.enabled === false) { + continue; + } + if (builtInRuleNames.includes(rule.$name) && !rule.custom) { // Built-in rule + if (rule.match || rule.expr_rule || rule.description) { + messages.push({ + rule: 'LAMS3', + level: 'info', + description: `Rule ${rule.$name} is a built-in rule. Some rule declaration properties (e.g. match, rule_expr) have been ignored.`, + }); + } + try { + let ruleModule = require('./rules/' + rule.$name.toLowerCase() + '.js'); + let result = ruleModule(project); + messages = messages.concat(result.messages); + } catch (e) { + messages.push({ + rule: 'LAMS1', + level: 'error', + description: `LAMS error evaluating rule ${rule.$name.toUpperCase()}: ${e.message || e}`, + }); + } + } else { + messages = messages.concat(checkCustomRule(rule, project, {console})); + } + } + console.log('> Rules done!'); - // Legacy custom rules - if (project.manifest && project.manifest.custom_rules) { - console.warn('\x1b[33m%s\x1b[0m', 'Legacy (Javascript) custom rules are unsafe and may be removed in a future major version!'); - console.log('For details, see https://looker-open-source.github.io/look-at-me-sideways/customizing-lams.html'); - if (options.allowCustomRules === undefined) { - console.warn([ - '> Your project specifies custom Javascript-based rules. Run LAMS with `--allow-custom-rules`', - 'if you want to allow potentially unsafe local execution of this remotely-defined Javascript code:', - ].concat(project.manifest.custom_rules).join('\n ')); - } else { - console.log('Checking legacy custom rules...'); - let get = options.get || require('./lib/https-get.js'); - let requireFromString = require('require-from-string'); - let customRuleRequests = []; - project.manifest.custom_rules.forEach(async (url, u) => { - try { - let request = get(url); - customRuleRequests.push(request); - let ruleSrc = await request; - console.log('> #' + u); - let rule = requireFromString(ruleSrc, { - prependPaths: path.resolve(__dirname, './rules'), - }); - let result = rule(project); - messages = messages.concat(result.messages.map((msg) => ({ - rule: `LCR ${u}`, - level: 'error', - ...msg, - }))); - } catch (e) { - let msg = `URL #${u}: ${e && e.message || e}`; - console.error('> ' + msg); - messages.push({ - rule: 'LAMS2', - level: 'error', - message: `An error occurred while checking legacy custom rule in ${msg}`, - }); - } - }); - await Promise.all(customRuleRequests).catch(() => { }); - console.log('> Legacy custom rules done!'); - } - } + // Legacy custom rules + if (project.manifest && project.manifest.custom_rules) { + console.warn('\x1b[33m%s\x1b[0m', 'Legacy (Javascript) custom rules are unsafe and may be removed in a future major version!'); + console.log('For details, see https://looker-open-source.github.io/look-at-me-sideways/customizing-lams.html'); + if (options.allowCustomRules === undefined) { + console.warn([ + '> Your project specifies custom Javascript-based rules. Run LAMS with `--allow-custom-rules`', + 'if you want to allow potentially unsafe local execution of this remotely-defined Javascript code:', + ].concat(project.manifest.custom_rules).join('\n ')); + } else { + console.log('Checking legacy custom rules...'); + let get = options.get || require('./lib/https-get.js'); + let requireFromString = require('require-from-string'); + let customRuleRequests = []; + project.manifest.custom_rules.forEach(async (url, u) => { + try { + let request = get(url); + customRuleRequests.push(request); + let ruleSrc = await request; + console.log('> #' + u); + let rule = requireFromString(ruleSrc, { + prependPaths: path.resolve(__dirname, './rules'), + }); + let result = rule(project); + messages = messages.concat(result.messages.map((msg) => ({ + rule: `LCR ${u}`, + level: 'error', + ...msg, + }))); + } catch (e) { + let msg = `URL #${u}: ${e && e.message || e}`; + console.error('> ' + msg); + messages.push({ + rule: 'LAMS2', + level: 'error', + message: `An error occurred while checking legacy custom rule in ${msg}`, + }); + } + }); + await Promise.all(customRuleRequests).catch(() => { }); + console.log('> Legacy custom rules done!'); + } + } - // Output - const outputters = require('./lib/outputters/index.js'); - let errors = messages.filter((msg) => { - return msg.level === 'error' && !msg.exempt; - }); + // Output + const outputters = require('./lib/outputters/index.js'); + let errors = messages.filter((msg) => { + return msg.level === 'error' && !msg.exempt; + }); - const outputModes = + const outputModes = options.jenkins ? 'jenkins,markdown' - : options.output ? options.output - : 'lines'; - for (let output of outputModes.split(',')) { - switch (output) { - case '': break; - case 'add-exemptions':{ - const lamsRuleExemptionsPath = options.lamsRuleExemptionsPath; - await outputters.addExemptions(messages, {cwd, console, lamsRuleExemptionsPath}) - break; - } - case 'markdown': { - const {dateOutput} = options; - await outputters.markdown(messages, {dateOutput,console}); - break; - } - case 'markdown-developer': - await outputters.markdownDeveloper(messages, {console}); - break; - case 'lines': { - const verbose = options.verbose || false; - await outputters.lines(messages, {verbose, console}); - break; - } - case 'legacy-cli': - await outputters.legacyCli(messages); - break; - default: - console.warn(`Unrecognized output mode: ${output}`); - } - } + : options.output ? options.output + : 'lines'; + for (let output of outputModes.split(',')) { + switch (output) { + case '': break; + case 'add-exemptions': { + const lamsRuleExemptionsPath = options.lamsRuleExemptionsPath; + await outputters.addExemptions(messages, {cwd, console, lamsRuleExemptionsPath}); + break; + } + case 'markdown': { + const {dateOutput} = options; + await outputters.markdown(messages, {dateOutput, console}); + break; + } + case 'markdown-developer': + await outputters.markdownDeveloper(messages, {console}); + break; + case 'lines': { + const verbose = options.verbose || false; + await outputters.lines(messages, {verbose, console}); + break; + } + case 'legacy-cli': + await outputters.legacyCli(messages); + break; + default: + console.warn(`Unrecognized output mode: ${output}`); + } + } - if (tracker.enabled) { - await Promise.race([ - tracker.track({messages, errors: []}), - new Promise((res) => setTimeout(res, 2000)), - ]); - } + if (tracker.enabled) { + await Promise.race([ + tracker.track({messages, errors: []}), + new Promise((res) => setTimeout(res, 2000)), + ]); + } - if (errors.length) { - process.exit(1); - } + if (errors.length) { + process.exit(1); + } - return messages; - } catch (e) { - try { - console.error(e); - if (!tracker.valid) { - throw new Error('Unknown error'); - } - if (tracker.enabled) { - e.isFatalError = true; - tracker.track({messages, errors: [e]}); - } else { - console.warn(`Error reporting is disabled. Run with --reporting=yes to report, or see PRIVACY.md for more info`); - } - } catch (e) { - console.error(e); - console.error(`Error reporting is not available . Please submit an issue to https://github.com/looker-open-source/look-at-me-sideways/issues`); - } - process.exit(1); - } + return messages; + } catch (e) { + try { + console.error(e); + if (!tracker.valid) { + throw new Error('Unknown error'); + } + if (tracker.enabled) { + e.isFatalError = true; + tracker.track({messages, errors: [e]}); + } else { + console.warn(`Error reporting is disabled. Run with --reporting=yes to report, or see PRIVACY.md for more info`); + } + } catch (e) { + console.error(e); + console.error(`Error reporting is not available . Please submit an issue to https://github.com/looker-open-source/look-at-me-sideways/issues`); + } + process.exit(1); + } - return; + return; }; diff --git a/lib/get-exemption-deep.js b/lib/get-exemption-deep.js index 23d01f5..7ab666a 100644 --- a/lib/get-exemption-deep.js +++ b/lib/get-exemption-deep.js @@ -1,35 +1,35 @@ const getExemption = require('./get-exemption.js'); -const formatLocation = require('./custom-rule/format-location.js') +const formatLocation = require('./custom-rule/format-location.js'); module.exports = function getExemptionDeep({match, rule}) { // Check match for being exempt and return early (without evaluating expression) if so const {path, project} = match; - + // Check the manifest for global exemptions const manifestExempt = getExemption(project && project.manifest, rule); - if(manifestExempt){ - return {match, exempt: manifestExempt} + if (manifestExempt) { + return {match, exempt: manifestExempt}; } // Check for central exemptions - if(project.centralExemptions){ - for(let p=0, P=path.length;p({ - exempt: exempt || getExemption(modelFragment[pathpart], rule), - modelFragment: modelFragment[pathpart], - }), - {modelFragment: project}, - ).exempt; + ({modelFragment, exempt}, pathpart)=>({ + exempt: exempt || getExemption(modelFragment[pathpart], rule), + modelFragment: modelFragment[pathpart], + }), + {modelFragment: project}, + ).exempt; if (localExempt) { return {