From 2737019b38bf42588a6d6a4f9fa5f97044e9e5ea Mon Sep 17 00:00:00 2001 From: Ari Perkkio Date: Sat, 7 Nov 2020 10:21:57 +0200 Subject: [PATCH] feat(results): include erroneous line and source to linter crashes --- docs/results-markdown.md | 48 ++++++++++++++++- docs/results-plaintext | 46 +++++++++++++++- lib/engine/types.d.ts | 1 + lib/engine/worker-task.ts | 70 +++++++++++++++++++------ lib/file-client/index.ts | 2 +- lib/file-client/result-templates.ts | 18 +++++-- lib/file-client/results-writer.ts | 1 + test/integration/integration.ci.test.ts | 4 ++ 8 files changed, 167 insertions(+), 23 deletions(-) diff --git a/docs/results-markdown.md b/docs/results-markdown.md index 35dafdbf..b400328c 100644 --- a/docs/results-markdown.md +++ b/docs/results-markdown.md @@ -29,12 +29,58 @@ export default function NestedGrid() { ``` +## Rule: no-reference-type-as-default-prop +- Message: `Cannot read property 'type' of null +Occurred while linting :7` +- Path: `uber/baseweb/documentation-site/examples/getting-started/usage.js` +- [Link](https://github.com/uber/baseweb/blob/HEAD/documentation-site/examples/getting-started/usage.js#L7) +```js + +import {StatefulInput} from 'baseui/input'; +import {useStyletron} from 'baseui'; + +export default function() { + const [css] = useStyletron(); + return ( +
:7 + at isReactComponentName (//GIT/eslint-plugin-react-test/eslint-plugin-react/lib/rules/no-reference-type-as-default-prop.js:24:15) + at FunctionDeclaration (//GIT/eslint-plugin-react-test/eslint-plugin-react/lib/rules/no-reference-type-as-default-prop.js:115:12) + at //GIT/eslint-remote-tester/node_modules/eslint/lib/linter/safe-emitter.js:45:58 + at Array.forEach () + at Object.emit (//GIT/eslint-remote-tester/node_modules/eslint/lib/linter/safe-emitter.js:45:38) + at NodeEventGenerator.applySelector (//GIT/eslint-remote-tester/node_modules/eslint/lib/linter/node-event-generator.js:254:26) + at NodeEventGenerator.applySelectors (//GIT/eslint-remote-tester/node_modules/eslint/lib/linter/node-event-generator.js:283:22) + at NodeEventGenerator.enterNode (//GIT/eslint-remote-tester/node_modules/eslint/lib/linter/node-event-generator.js:297:14) + at CodePathAnalyzer.enterNode (//GIT/eslint-remote-tester/node_modules/eslint/lib/linter/code-path-analysis/code-path-analyzer.js:711:23) + at //GIT/eslint-remote-tester/node_modules/eslint/lib/linter/linter.js:952:32 +``` + ## Rule: jsx-handler-names - Message: `Cannot read property 'object' of undefined Occurred while linting :15` - Path: `mui-org/material-ui/docs/src/modules/components/AdGuest.js` -- [Link](https://github.com/mui-org/material-ui/blob/HEAD/docs/src/modules/components/AdGuest.js#L0) +- [Link](https://github.com/mui-org/material-ui/blob/HEAD/docs/src/modules/components/AdGuest.js#L15) ```js + return null; + } + + return ( + { + const description = document.querySelector('.description'); + + if (ad.portal.element === description) { + description.classList.add('ad'); + } else { +``` + +``` TypeError: Cannot read property 'object' of undefined Occurred while linting :15 at JSXAttribute (//eslint-plugin-react/lib/rules/jsx-handler-names.js:112:52) diff --git a/docs/results-plaintext b/docs/results-plaintext index 8c691bce..670504c3 100644 --- a/docs/results-plaintext +++ b/docs/results-plaintext @@ -27,11 +27,54 @@ export default function NestedGrid() { +Rule: no-reference-type-as-default-prop +Message: Cannot read property 'type' of null +Occurred while linting :5 +Path: uber/baseweb/documentation-site/examples/block/basic.tsx +Link: https://github.com/uber/baseweb/blob/HEAD/documentation-site/examples/block/basic.tsx#L5 + +// @flow +import React from 'react'; +import {Block} from 'baseui/block'; + +export default function() { + return ( + + first element + + padding top applied to this element + +TypeError: Cannot read property 'type' of null +Occurred while linting :7 + at isReactComponentName (//eslint-plugin-react/lib/rules/no-reference-type-as-default-prop.js:24:15) + at FunctionDeclaration (//eslint-plugin-react/lib/rules/no-reference-type-as-default-prop.js:115:12) + at //eslint-remote-tester/node_modules/eslint/lib/linter/safe-emitter.js:45:58 + at Array.forEach () + at Object.emit (//eslint-remote-tester/node_modules/eslint/lib/linter/safe-emitter.js:45:38) + at NodeEventGenerator.applySelector (//eslint-remote-tester/node_modules/eslint/lib/linter/node-event-generator.js:254:26) + at NodeEventGenerator.applySelectors (//eslint-remote-tester/node_modules/eslint/lib/linter/node-event-generator.js:283:22) + at NodeEventGenerator.enterNode (//eslint-remote-tester/node_modules/eslint/lib/linter/node-event-generator.js:297:14) + at CodePathAnalyzer.enterNode (//eslint-remote-tester/node_modules/eslint/lib/linter/code-path-analysis/code-path-analyzer.js:711:23) + at //eslint-remote-tester/node_modules/eslint/lib/linter/linter.js:952:32 + Rule: jsx-handler-names Message: Cannot read property 'object' of undefined Occurred while linting :15 Path: mui-org/material-ui/docs/src/modules/components/AdGuest.js -Link: https://github.com/mui-org/material-ui/blob/HEAD/docs/src/modules/components/AdGuest.js#L0 +Link: https://github.com/mui-org/material-ui/blob/HEAD/docs/src/modules/components/AdGuest.js#L15 + + return null; + } + + return ( + { + const description = document.querySelector('.description'); + + if (ad.portal.element === description) { + description.classList.add('ad'); + } else { + TypeError: Cannot read property 'object' of undefined Occurred while linting :15 at JSXAttribute (//eslint-plugin-react/lib/rules/jsx-handler-names.js:112:52) @@ -49,6 +92,7 @@ Rule: react/jsx-handler-names Message: Handler function for onClick prop key must be a camelCase name beginning with 'handle' only Path: mui-org/material-ui/docs/src/pages/components/accordion/ActionsInAccordionSummary.js Link: https://github.com/mui-org/material-ui/blob/HEAD/docs/src/pages/components/accordion/ActionsInAccordionSummary.js#L31-L31 + id="additional-actions1-header" > :(([0-9]+)?)/; + const SOURCE_WINDOW_SIZE = 10; const MAX_LINT_TIME_SECONDS = 5; @@ -29,12 +33,12 @@ const MAX_LINT_TIME_SECONDS = 5; * Create error message for LintMessage results */ export function createErrorMessage( - error: Omit + error: Omit & { line?: number } ): LintMessage { return { + line: 0, ...error, column: 0, - line: 0, severity: 0, }; } @@ -94,6 +98,45 @@ function mergeMessagesWithSource( ]; } +/** + * Parse error stack for erroneous lines and construct `LintMessage` with + * source code. + */ +function parseErrorStack(error: Error, file: SourceFile): LintMessage { + const { content, path } = file; + + const stack = error.stack || ''; + const ruleMatch = stack.match(RULE_REGEXP) || []; + const ruleId = ruleMatch.pop() || null; + + const lineMatch = stack.match(LINE_REGEX) || []; + const line = parseInt(lineMatch.pop() || '0'); + + const sourceLines = []; + + // Include erroneous line to source when line was successfully parsed from the stack + if (line > 0) { + const sourceLinesPadding = Math.floor(SOURCE_WINDOW_SIZE / 2); + const lines = content.split('\n'); + + sourceLines.push( + ...lines.slice( + Math.max(0, line - sourceLinesPadding), + line + sourceLinesPadding + ) + ); + } + + return createErrorMessage({ + path, + line, + ruleId, + source: sourceLines.join('\n'), + error: error.stack, + message: error.message, + }); +} + // Wrapper used to enfore WorkerMessage type to parentPort.postMessage calls const postMessage = (message: WorkerMessage) => { if (parentPort) { @@ -150,19 +193,14 @@ export default async function workerTask(): Promise { ); } catch (error) { // Catch crashing linter - const stack = error.stack || ''; - const ruleMatch = stack.match(RULE_REGEXP) || []; - const ruleId = ruleMatch.pop(); - - results.push( - createErrorMessage({ - path, - message: error.message, - source: error.stack, - ruleId, - }) - ); - postMessage({ type: 'LINTER_CRASH', payload: ruleId }); + const crashError = parseErrorStack(error, file); + + results.push(crashError); + postMessage({ + type: 'LINTER_CRASH', + payload: crashError.ruleId || '', + }); + continue; } diff --git a/lib/file-client/index.ts b/lib/file-client/index.ts index 6ad63c64..e28396b2 100644 --- a/lib/file-client/index.ts +++ b/lib/file-client/index.ts @@ -1,4 +1,4 @@ -export { getFiles } from './file-client'; +export { getFiles, SourceFile } from './file-client'; export { writeResults, getResults, diff --git a/lib/file-client/result-templates.ts b/lib/file-client/result-templates.ts index 7b893a3c..22615b37 100644 --- a/lib/file-client/result-templates.ts +++ b/lib/file-client/result-templates.ts @@ -8,25 +8,35 @@ interface ResultTemplateOptions { link: string; extension?: string; source: LintMessage['source']; + error?: LintMessage['error']; } +// prettier-ignore const RESULT_TEMPLATE_PLAINTEXT = (options: ResultTemplateOptions): string => - `Rule: ${options.rule} +`Rule: ${options.rule} Message: ${options.message} Path: ${options.path} Link: ${options.link} + ${options.source} -`; +${options.error ? ` +Error: +${options.error} +` : ''}`; +// prettier-ignore const RESULT_TEMPLATE_MARKDOWN = (options: ResultTemplateOptions): string => - `## Rule: ${options.rule} +`## Rule: ${options.rule} - Message: \`${options.message}\` - Path: \`${options.path}\` - [Link](${options.link}) \`\`\`${options.extension || ''} ${options.source || ''} \`\`\` -`; +${options.error ? +`\`\`\` +${options.error} +\`\`\`` : ''}`; export const RESULT_PARSER_TO_TEMPLATE: Record< ResultParser, diff --git a/lib/file-client/results-writer.ts b/lib/file-client/results-writer.ts index ae94d3c0..dc61c3ee 100644 --- a/lib/file-client/results-writer.ts +++ b/lib/file-client/results-writer.ts @@ -46,6 +46,7 @@ const RESULT_TEMPLATE_CLI = (result: LintMessage) => { link: `${URL}/${project}/${repository}${postfix}`, extension, source: result.source, + error: result.error, }); }; diff --git a/test/integration/integration.ci.test.ts b/test/integration/integration.ci.test.ts index 4567944c..7738a8e2 100644 --- a/test/integration/integration.ci.test.ts +++ b/test/integration/integration.ci.test.ts @@ -25,6 +25,7 @@ describe('CI mode', () => { Message: 'bar' is not defined. Path: AriPerkkio/eslint-remote-tester-integration-test-target/index.js Link: https://github.com/AriPerkkio/eslint-remote-tester-integration-test-target/blob/HEAD/index.js#L1-L1 + var foo = bar; if (foo) { @@ -36,6 +37,7 @@ describe('CI mode', () => { Message: Empty block statement. Path: AriPerkkio/eslint-remote-tester-integration-test-target/index.js Link: https://github.com/AriPerkkio/eslint-remote-tester-integration-test-target/blob/HEAD/index.js#L3-L4 + var foo = bar; if (foo) { @@ -50,6 +52,7 @@ describe('CI mode', () => { Message: Expected to return a value in getter 'name'. Path: AriPerkkio/eslint-remote-tester-integration-test-target/index.js Link: https://github.com/AriPerkkio/eslint-remote-tester-integration-test-target/blob/HEAD/index.js#L7-L7 + if (foo) { } @@ -65,6 +68,7 @@ describe('CI mode', () => { Message: Do not use the '===' operator to compare against -0. Path: AriPerkkio/eslint-remote-tester-integration-test-target/index.js Link: https://github.com/AriPerkkio/eslint-remote-tester-integration-test-target/blob/HEAD/index.js#L14-L14 + }; p.getName();