diff --git a/README.md b/README.md index c96ba98..e640a0e 100644 --- a/README.md +++ b/README.md @@ -53,9 +53,13 @@ As of LAMS v3, you must opt-in via your `manifest.lkml` file to use the built-in #rule: W1{} # Block indentation ``` +### 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: @@ -71,9 +75,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 @@ -116,7 +126,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/__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..3d0c6c1 --- /dev/null +++ b/__tests__/dummy-projects/27-output-add-exemptions/index.test.js @@ -0,0 +1,90 @@ +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"}\n'; +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") + 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/__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/docs/github-action.md b/docs/github-action.md index d7149fe..86a2225 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 `lams-exemptions.ndjson` 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/index.js b/index.js index f6421f1..d9ec369 100755 --- a/index.js +++ b/index.js @@ -34,376 +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!'); - // 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?'...':''}`}); + } - project.name = false + // 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('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()); + console.log('> Manifest and exemptions done'); - 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.'); - } + console.log('Checking rules... '); - 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!'); + 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()); - // 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!'); - } - } + 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.'); + } - // Output - let errors = messages.filter((msg) => { - return msg.level === 'error' && !msg.exempt; - }); - - const outputModes = - options.jenkins ? 'jenkins,markdown' - : options.output ? options.output - : 'lines'; - for (let output of outputModes.split(',')) { - switch (output) { - case '': break; - case 'markdown': { - const {dateOutput} = options; - await outputMarkdown(messages, {dateOutput}); - break; - } - case 'markdown-developer': - await outputDeveloperMarkdown(messages); - break; - case 'jenkins': - await outputJenkins(messages); - break; - case 'lines': { - const verbose = options.verbose || false; - await outputLines(messages, {verbose}); - break; - } - case 'legacy-cli': - await outputLegacyCli(messages); - break; - default: - console.warn(`Unrecognized output mode: ${output}`); - } - } + 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!'); - if (tracker.enabled) { - await Promise.race([ - tracker.track({messages, errors: []}), - new Promise((res) => setTimeout(res, 2000)), - ]); - } + // 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!'); + } + } - if (errors.length) { - process.exit(1); - } + // Output + const outputters = require('./lib/outputters/index.js'); + let errors = messages.filter((msg) => { + return msg.level === 'error' && !msg.exempt; + }); - 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; - - /** - * 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'); - } + 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}`); + } + } - /** - * 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'); - } + if (tracker.enabled) { + await Promise.race([ + tracker.track({messages, errors: []}), + new Promise((res) => setTimeout(res, 2000)), + ]); + } - /** - * 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'); - } + if (errors.length) { + process.exit(1); + } - /** - * 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}); - } - } + 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); + } - /** - * 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')); - } - } + return; }; 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..7ab666a 100644 --- a/lib/get-exemption-deep.js +++ b/lib/get-exemption-deep.js @@ -1,23 +1,40 @@ 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( - ({modelFragment, exempt}, pathpart)=>({ - exempt: exempt || getExemption(modelFragment[pathpart], rule), - modelFragment: modelFragment[pathpart], - }), - {modelFragment: project}, - ).exempt; + 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; - 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..ec1a73c --- /dev/null +++ b/lib/loaders/lams-exemptions.js @@ -0,0 +1,78 @@ +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 + ) + } + + if(centralExemptions.size>0){ + console.log("BETA: lams-exemptions.ndjson and incremental linting are in beta. Feedback welcome in LAMS issue #142.") + } + + 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 new file mode 100644 index 0000000..fee5760 --- /dev/null +++ b/lib/outputters/add-exemptions.js @@ -0,0 +1,55 @@ +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, { + cwd = process.cwd(), + lamsRuleExemptionsPath = "lams-exemptions.ndjson", + console = defaultConsole, + 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 + } + + const centralExemptionsPath = pathLib.resolve(cwd, lamsRuleExemptionsPath) + + // Append error messages into lams-exemptions.ndjson + let file + try{ + file = await fs.open(centralExemptionsPath,"a") + + 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 new file mode 100644 index 0000000..b716980 --- /dev/null +++ b/lib/outputters/index.js @@ -0,0 +1,10 @@ +const outputters = { + addExemptions: require('./add-exemptions'), + jenkins: require("./jenkins.js"), + legacyCli: require("./legacy-cli.js"), + lines: require("./lines.js"), + markdown: require("./markdown.js"), + markdownDeveloper: require("./markdown-developer.js"), +}; + +module.exports = outputters; 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 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 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; 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 new file mode 100644 index 0000000..ec76eb6 --- /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 = {},