From 4688474b2ca1f9a0951a35d99bc329e97e92e89c Mon Sep 17 00:00:00 2001 From: Mikael Vesavuori Date: Sat, 8 Jul 2023 13:55:24 +0200 Subject: [PATCH] feat(): add checkForThrowingPlainErrors; add support for ignore paths; fix bug where path was never forwarded to final check --- README.md | 4 +- package-lock.json | 4 +- package.json | 2 +- src/checks/checkForConsoleUsage.ts | 12 +++-- src/checks/checkForPresenceTests.ts | 8 +++- src/checks/checkForThrowingPlainErrors.ts | 39 +++++++++++++++ src/domain/StandardLint.ts | 47 ++++++++++++++----- src/interface/Check.ts | 7 +++ src/interface/StandardLint.ts | 7 ++- src/utils/filterFiles.ts | 11 +++++ tests/checks/checkForConsoleUsage.test.ts | 15 +++++- .../checkForThrowingPlainErrors.test.ts | 35 ++++++++++++++ tests/domain/StandardLint.ts | 32 +++++++++++++ 13 files changed, 199 insertions(+), 24 deletions(-) create mode 100644 src/checks/checkForThrowingPlainErrors.ts create mode 100644 src/utils/filterFiles.ts create mode 100644 tests/checks/checkForThrowingPlainErrors.test.ts diff --git a/README.md b/README.md index 4975a78..d4ef1bf 100644 --- a/README.md +++ b/README.md @@ -257,11 +257,11 @@ Checks if there are any tests in the provided location. This will match anything Checks: - Do: + - Check for throwing native errors + - Ensure methods/functions are below a certain threshold in terms of lines of code - Documentation coverage - Ensure imports follow acceptable conventions - No cyclic methods/dependencies - - Ensure methods/functions are below a certain threshold in terms of lines of code - - Check for throwing native errors - Do later: - Service metadata: Do you link to observability resources (logs/metrics/traces/dashboards etc.)? - Support for external config diff --git a/package-lock.json b/package-lock.json index b6ea905..06d635f 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "standardlint", - "version": "1.0.4", + "version": "1.0.5", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "standardlint", - "version": "1.0.4", + "version": "1.0.5", "license": "MIT", "bin": { "standardlint": "dist/index.js" diff --git a/package.json b/package.json index 96bbbf4..77fcb2a 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "standardlint", "description": "Extensible standards linter and auditor.", - "version": "1.0.4", + "version": "1.0.5", "author": "Mikael Vesavuori", "license": "MIT", "keywords": [ diff --git a/src/checks/checkForConsoleUsage.ts b/src/checks/checkForConsoleUsage.ts index b126d12..2f61054 100644 --- a/src/checks/checkForConsoleUsage.ts +++ b/src/checks/checkForConsoleUsage.ts @@ -5,6 +5,7 @@ import { calculatePass } from '../application/calculatePass'; import { logDefaultPathMessage } from '../utils/logDefaultPathMessage'; import { getAllFiles } from '../utils/getAllFiles'; import { readFile } from '../utils/readFile'; +import { filterFiles } from '../utils/filterFiles'; /** * @description Checks if there is an API schema. @@ -12,7 +13,8 @@ import { readFile } from '../utils/readFile'; export function checkForConsoleUsage( severity: Severity, basePath: string, - customPath?: string + customPath?: string, + ignorePaths?: string[] ): CheckResult { const path = customPath || 'src'; const name = 'Console usage'; @@ -20,10 +22,12 @@ export function checkForConsoleUsage( if (!customPath) logDefaultPathMessage(name, path); - const tests = getAllFiles(`${basePath}/${path}`, []); + const files = getAllFiles(`${basePath}/${path}`, []); + const filteredFiles = ignorePaths ? filterFiles(files, ignorePaths) : files; + const regex = /console.(.*)/gi; - const includesConsole = tests.map((test: string) => regex.test(readFile(test))); - const result = !includesConsole.some(() => true); // We don't want any occurrences + const includesConsole = filteredFiles.map((test: string) => regex.test(readFile(test))); + const result = !includesConsole.includes(true); // We don't want any occurrences return { name, diff --git a/src/checks/checkForPresenceTests.ts b/src/checks/checkForPresenceTests.ts index c392657..f6e9f65 100644 --- a/src/checks/checkForPresenceTests.ts +++ b/src/checks/checkForPresenceTests.ts @@ -4,6 +4,7 @@ import { calculatePass } from '../application/calculatePass'; import { logDefaultPathMessage } from '../utils/logDefaultPathMessage'; import { getAllFiles } from '../utils/getAllFiles'; +import { filterFiles } from '../utils/filterFiles'; /** * @description Checks if there are tests. @@ -11,7 +12,8 @@ import { getAllFiles } from '../utils/getAllFiles'; export function checkForPresenceTests( severity: Severity, basePath: string, - customPath?: string + customPath?: string, + ignorePaths?: string[] ): CheckResult { const path = customPath || 'tests'; const name = 'Tests'; @@ -20,7 +22,9 @@ export function checkForPresenceTests( if (!customPath) logDefaultPathMessage(name, path); const files = getAllFiles(`${basePath}/${path}`, []); - const tests = files.filter( + const filteredFiles = ignorePaths ? filterFiles(files, ignorePaths) : files; + + const tests = filteredFiles.filter( (file: string) => file.endsWith('test.ts') || file.endsWith('spec.ts') || diff --git a/src/checks/checkForThrowingPlainErrors.ts b/src/checks/checkForThrowingPlainErrors.ts new file mode 100644 index 0000000..b7e37e3 --- /dev/null +++ b/src/checks/checkForThrowingPlainErrors.ts @@ -0,0 +1,39 @@ +import { CheckResult, Severity } from '../interface/Check'; + +import { calculatePass } from '../application/calculatePass'; + +import { logDefaultPathMessage } from '../utils/logDefaultPathMessage'; +import { getAllFiles } from '../utils/getAllFiles'; +import { readFile } from '../utils/readFile'; +import { filterFiles } from '../utils/filterFiles'; + +/** + * @description Checks if plain (non-custom) errors are thrown. + */ +export function checkForThrowingPlainErrors( + severity: Severity, + basePath: string, + customPath?: string, + ignorePaths?: string[] +): CheckResult { + const path = customPath || 'src'; + const name = 'Error handling'; + const message = 'Check for presence of plain (non-custom) errors'; + + if (!customPath) logDefaultPathMessage(name, path); + + const files = getAllFiles(`${basePath}/${path}`, []); + const filteredFiles = ignorePaths ? filterFiles(files, ignorePaths) : files; + + const regex = /(throw Error|throw new Error)(.*)/gi; + const includesError = filteredFiles.map((test: string) => regex.test(readFile(test))); + + const result = !includesError.includes(true); + + return { + name, + status: calculatePass(result, severity), + message, + path + }; +} diff --git a/src/domain/StandardLint.ts b/src/domain/StandardLint.ts index f3a490d..0b887d0 100644 --- a/src/domain/StandardLint.ts +++ b/src/domain/StandardLint.ts @@ -1,4 +1,4 @@ -import { CheckResult, Check, Severity, CheckInput } from '../interface/Check'; +import { CheckResult, Check, Severity, CheckInput, IgnorePath } from '../interface/Check'; import { Configuration, ConfigurationInput, Result } from '../interface/StandardLint'; import { getStatusCount } from '../application/getStatusCount'; @@ -22,6 +22,7 @@ import { checkForPresenceServiceMetadata } from '../checks/checkForPresenceServi import { checkForPresenceTemplateIssues } from '../checks/checkForPresenceTemplateIssues'; import { checkForPresenceTemplatePullRequests } from '../checks/checkForPresenceTemplatePullRequests'; import { checkForPresenceTests } from '../checks/checkForPresenceTests'; +import { checkForThrowingPlainErrors } from '../checks/checkForThrowingPlainErrors'; import { exists } from '../utils/exists'; @@ -41,6 +42,7 @@ export function createNewStandardLint(config?: ConfigurationInput) { class StandardLint { private readonly defaultBasePathFallback = '.'; private readonly defaultSeverityFallback = 'error'; + private readonly defaultIgnorePathsFallback = []; readonly config: Configuration; constructor(config?: ConfigurationInput) { @@ -60,8 +62,12 @@ class StandardLint { ? this.getValidatedSeverityLevel(configInput.defaultSeverity) : this.defaultSeverityFallback; + const ignorePaths = configInput?.ignorePaths + ? this.getSanitizedPaths(configInput.ignorePaths) + : this.defaultIgnorePathsFallback; + const checkList = Array.isArray(configInput?.checks) ? configInput?.checks : []; - const checks = this.getValidatedChecks(checkList as CheckInput[], defaultSeverity); + const checks = this.getValidatedChecks(checkList as CheckInput[], defaultSeverity, ignorePaths); return { basePath, @@ -80,13 +86,25 @@ class StandardLint { return this.defaultSeverityFallback; } + /** + * @description Sanitizes provided paths or return empty array if none is provided. + */ + private getSanitizedPaths(ignorePaths: IgnorePath[]) { + if (ignorePaths.length === 0) return []; + return ignorePaths.filter((path: string) => typeof path === 'string'); + } + /** * @description Validates and sanitizes a requested list of checks. * * Provide `defaultSeverity` as it's not yet available in the class `config` object * when running the validation. */ - private getValidatedChecks(checks: (string | CheckInput)[], defaultSeverity: Severity): Check[] { + private getValidatedChecks( + checks: (string | CheckInput)[], + defaultSeverity: Severity, + ignorePaths: IgnorePath[] + ): Check[] { const validCheckNames = [ 'all', 'checkForConflictingLockfiles', @@ -107,7 +125,8 @@ class StandardLint { 'checkForPresenceServiceMetadata', 'checkForPresenceTemplateIssues', 'checkForPresenceTemplatePullRequests', - 'checkForPresenceTests' + 'checkForPresenceTests', + 'checkForThrowingPlainErrors' ]; const isValidCheckName = (name: string) => validCheckNames.includes(name); @@ -122,19 +141,21 @@ class StandardLint { if (typeof check === 'string' && isValidCheckName(check)) return { name: check, - severity: defaultSeverity + severity: defaultSeverity, + ignorePaths }; if (typeof check === 'object' && isValidCheckName(check.name)) return { name: check.name, - severity: this.getValidatedSeverityLevel(check.severity || defaultSeverity) + path: check.path || '', + severity: this.getValidatedSeverityLevel(check.severity || defaultSeverity), + ignorePaths }; // No match, remove in filter step return { - name: '', - severity: defaultSeverity + name: '' }; }) .filter((check: CheckInput) => check.name); @@ -166,12 +187,13 @@ class StandardLint { * @description Run test on an individual Check. */ private test(check: Check): CheckResult { - const { name, severity, path } = check; + const { name, severity, path, ignorePaths } = check; const checksList: any = { checkForConflictingLockfiles: () => checkForConflictingLockfiles(severity, this.config.basePath), - checkForConsoleUsage: () => checkForConsoleUsage(severity, this.config.basePath, path), + checkForConsoleUsage: () => + checkForConsoleUsage(severity, this.config.basePath, path, ignorePaths), checkForDefinedRelations: () => checkForDefinedRelations(severity, this.config.basePath, path), checkForDefinedServiceLevelObjectives: () => @@ -198,7 +220,10 @@ class StandardLint { checkForPresenceTemplateIssues(severity, this.config.basePath, path), checkForPresenceTemplatePullRequests: () => checkForPresenceTemplatePullRequests(severity, this.config.basePath, path), - checkForPresenceTests: () => checkForPresenceTests(severity, this.config.basePath, path) + checkForPresenceTests: () => + checkForPresenceTests(severity, this.config.basePath, path, ignorePaths), + checkForThrowingPlainErrors: () => + checkForThrowingPlainErrors(severity, this.config.basePath, path, ignorePaths) }; const result = checksList[name](); diff --git a/src/interface/Check.ts b/src/interface/Check.ts index 65102f5..1dcc823 100644 --- a/src/interface/Check.ts +++ b/src/interface/Check.ts @@ -5,6 +5,7 @@ export type CheckInput = { name: string; severity?: Severity; path?: string; + ignorePaths?: IgnorePath[]; }; /** @@ -14,6 +15,7 @@ export type Check = { name: string; severity: Severity; path: string; + ignorePaths: IgnorePath[]; }; /** @@ -35,3 +37,8 @@ export type Status = 'pass' | 'warn' | 'fail'; * @description Represents how severe a non-passing state is. */ export type Severity = 'error' | 'warn'; + +/** + * @description A path that will be ignored during certain checks. + */ +export type IgnorePath = string; diff --git a/src/interface/StandardLint.ts b/src/interface/StandardLint.ts index 8c5b546..5c08a5e 100644 --- a/src/interface/StandardLint.ts +++ b/src/interface/StandardLint.ts @@ -1,4 +1,4 @@ -import { Check, CheckInput, CheckResult, Severity } from './Check'; +import { Check, CheckInput, CheckResult, IgnorePath, Severity } from './Check'; /** * @description The `StandardLint` configuration. @@ -17,6 +17,10 @@ export type Configuration = { * default fallback level. */ defaultSeverity: Severity; + /** + * @description For some checks, the user can choose to ignore files on these paths. + */ + ignorePaths: IgnorePath[]; }; /** @@ -26,6 +30,7 @@ export type ConfigurationInput = { basePath?: string; checks?: (string | CheckInput)[]; defaultSeverity?: Severity; + ignorePaths?: IgnorePath[]; }; /** diff --git a/src/utils/filterFiles.ts b/src/utils/filterFiles.ts new file mode 100644 index 0000000..8f39822 --- /dev/null +++ b/src/utils/filterFiles.ts @@ -0,0 +1,11 @@ +/** + * @description Filters out file paths based on provided ignore paths. + */ +export function filterFiles(files: string[], ignorePaths: string[]) { + return files.filter((file: string) => + ignorePaths.every((ignorePath: string) => { + const localFilePath = file.replace(process.cwd(), ''); // This is so we don't catch on other file paths in the system + return !localFilePath.includes(ignorePath); + }) + ); +} diff --git a/tests/checks/checkForConsoleUsage.test.ts b/tests/checks/checkForConsoleUsage.test.ts index ac569c2..c4af228 100644 --- a/tests/checks/checkForConsoleUsage.test.ts +++ b/tests/checks/checkForConsoleUsage.test.ts @@ -26,8 +26,21 @@ test('It should pass when not finding any console usage when using check-only pa t.deepEqual(result, expected); }); +test('It should pass and accept ignore paths', (t) => { + const expected = 'pass'; + + const standardlint = createNewStandardLint({ + basePath: 'testdata', + ignorePaths: ['/src/'], + checks: [{ name: 'checkForConsoleUsage' }] + }); + const result = standardlint.check().results?.[0]?.status; + + t.deepEqual(result, expected); +}); + /** - * NEGATIVE CASES + * NEGATIVE TESTS */ test('It should fail when finding console.log()', (t) => { diff --git a/tests/checks/checkForThrowingPlainErrors.test.ts b/tests/checks/checkForThrowingPlainErrors.test.ts new file mode 100644 index 0000000..5997c7e --- /dev/null +++ b/tests/checks/checkForThrowingPlainErrors.test.ts @@ -0,0 +1,35 @@ +import test from 'ava'; + +import { createNewStandardLint } from '../../src/domain/StandardLint'; + +test('It should pass when not finding any plain errors being thrown', (t) => { + const expected = 'pass'; + + const standardlint = createNewStandardLint({ + basePath: '', + checks: [ + { + name: 'checkForThrowingPlainErrors', + path: 'tests' + } + ] + }); + const result = standardlint.check().results?.[0]?.status; + + t.deepEqual(result, expected); +}); + +/** + * NEGATIVE TESTS + */ + +test('It should fail if it finds a plain error being thrown', (t) => { + const expected = 'fail'; + + const standardlint = createNewStandardLint({ + checks: ['checkForThrowingPlainErrors'] + }); + const result = standardlint.check().results?.[0]?.status; + + t.deepEqual(result, expected); +}); diff --git a/tests/domain/StandardLint.ts b/tests/domain/StandardLint.ts index c36e816..c1edbc0 100644 --- a/tests/domain/StandardLint.ts +++ b/tests/domain/StandardLint.ts @@ -63,6 +63,8 @@ test('It should set the default severity to the fallback value if provided value test('It should use a fallback severity value if an invalid severity value is encountered', (t) => { const expected = [ { + path: '', + ignorePaths: [], name: 'checkForPresenceContributing', severity: 'error' } @@ -80,6 +82,7 @@ test('It should use a fallback severity value if an invalid severity value is en test('It should validate a single valid check', (t) => { const expected = [ { + ignorePaths: [], name: 'checkForPresenceContributing', severity: 'error' } @@ -96,6 +99,7 @@ test('It should validate a single valid check', (t) => { test('It should remove checks with unknown names', (t) => { const expected = [ { + ignorePaths: [], name: 'checkForPresenceContributing', severity: 'error' } @@ -112,11 +116,14 @@ test('It should remove checks with unknown names', (t) => { test('It should validate a mixed set of string and object-defined checks', (t) => { const expected = [ { + ignorePaths: [], name: 'checkForPresenceContributing', severity: 'error' }, { + ignorePaths: [], name: 'checkForPresenceLicense', + path: '', severity: 'warn' } ]; @@ -125,6 +132,7 @@ test('It should validate a mixed set of string and object-defined checks', (t) = checks: [ 'checkForPresenceContributing', { + ignorePaths: [], name: 'checkForPresenceLicense', severity: 'warn' } @@ -138,80 +146,104 @@ test('It should validate a mixed set of string and object-defined checks', (t) = test('It should use all checks if provided the "all" check option', (t) => { const expected = [ { + ignorePaths: [], name: 'checkForConflictingLockfiles', severity: 'error' }, { + ignorePaths: [], name: 'checkForConsoleUsage', severity: 'error' }, { + ignorePaths: [], name: 'checkForDefinedRelations', severity: 'error' }, { + ignorePaths: [], name: 'checkForDefinedServiceLevelObjectives', severity: 'error' }, { + ignorePaths: [], name: 'checkForDefinedTags', severity: 'error' }, { + ignorePaths: [], name: 'checkForPresenceApiSchema', severity: 'error' }, { + ignorePaths: [], name: 'checkForPresenceChangelog', severity: 'error' }, { + ignorePaths: [], name: 'checkForPresenceCiConfig', severity: 'error' }, { + ignorePaths: [], name: 'checkForPresenceCodeowners', severity: 'error' }, { + ignorePaths: [], name: 'checkForPresenceContributing', severity: 'error' }, { + ignorePaths: [], name: 'checkForPresenceDiagramsFolder', severity: 'error' }, { + ignorePaths: [], name: 'checkForPresenceIacConfig', severity: 'error' }, { + ignorePaths: [], name: 'checkForPresenceLicense', severity: 'error' }, { + ignorePaths: [], name: 'checkForPresenceReadme', severity: 'error' }, { + ignorePaths: [], name: 'checkForPresenceSecurity', severity: 'error' }, { + ignorePaths: [], name: 'checkForPresenceServiceMetadata', severity: 'error' }, { + ignorePaths: [], name: 'checkForPresenceTemplateIssues', severity: 'error' }, { + ignorePaths: [], name: 'checkForPresenceTemplatePullRequests', severity: 'error' }, { + ignorePaths: [], name: 'checkForPresenceTests', severity: 'error' + }, + { + ignorePaths: [], + name: 'checkForThrowingPlainErrors', + severity: 'error' } ];