Skip to content

Commit

Permalink
feat(results): include erroneous line and source to linter crashes
Browse files Browse the repository at this point in the history
  • Loading branch information
AriPerkkio committed Nov 7, 2020
1 parent caa6236 commit 2737019
Show file tree
Hide file tree
Showing 8 changed files with 167 additions and 23 deletions.
48 changes: 47 additions & 1 deletion docs/results-markdown.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,58 @@ export default function NestedGrid() {
<Grid container item xs={12} spacing={3}>
```
## Rule: no-reference-type-as-default-prop
- Message: `Cannot read property 'type' of null
Occurred while linting <text>: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 (
<div
className={css({
display: 'flex',
```
```
TypeError: Cannot read property 'type' of null
Occurred while linting <text>:7
at isReactComponentName (/<removed>/GIT/eslint-plugin-react-test/eslint-plugin-react/lib/rules/no-reference-type-as-default-prop.js:24:15)
at FunctionDeclaration (/<removed>/GIT/eslint-plugin-react-test/eslint-plugin-react/lib/rules/no-reference-type-as-default-prop.js:115:12)
at /<removed>/GIT/eslint-remote-tester/node_modules/eslint/lib/linter/safe-emitter.js:45:58
at Array.forEach (<anonymous>)
at Object.emit (/<removed>/GIT/eslint-remote-tester/node_modules/eslint/lib/linter/safe-emitter.js:45:38)
at NodeEventGenerator.applySelector (/<removed>/GIT/eslint-remote-tester/node_modules/eslint/lib/linter/node-event-generator.js:254:26)
at NodeEventGenerator.applySelectors (/<removed>/GIT/eslint-remote-tester/node_modules/eslint/lib/linter/node-event-generator.js:283:22)
at NodeEventGenerator.enterNode (/<removed>/GIT/eslint-remote-tester/node_modules/eslint/lib/linter/node-event-generator.js:297:14)
at CodePathAnalyzer.enterNode (/<removed>/GIT/eslint-remote-tester/node_modules/eslint/lib/linter/code-path-analysis/code-path-analyzer.js:711:23)
at /<removed>/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 <text>: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 (
<Portal
container={() => {
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 <text>:15
at JSXAttribute (/<removed>/eslint-plugin-react/lib/rules/jsx-handler-names.js:112:52)
Expand Down
46 changes: 45 additions & 1 deletion docs/results-plaintext
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,54 @@ export default function NestedGrid() {
<Grid container spacing={1}>
<Grid container item xs={12} spacing={3}>

Rule: no-reference-type-as-default-prop
Message: Cannot read property 'type' of null
Occurred while linting <text>: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 (
<React.Fragment>
<Block>first element</Block>
<Block paddingTop="100px">
padding top applied to this element

TypeError: Cannot read property 'type' of null
Occurred while linting <text>:7
at isReactComponentName (/<removed>/eslint-plugin-react/lib/rules/no-reference-type-as-default-prop.js:24:15)
at FunctionDeclaration (/<removed>/eslint-plugin-react/lib/rules/no-reference-type-as-default-prop.js:115:12)
at /<removed>/eslint-remote-tester/node_modules/eslint/lib/linter/safe-emitter.js:45:58
at Array.forEach (<anonymous>)
at Object.emit (/<removed>/eslint-remote-tester/node_modules/eslint/lib/linter/safe-emitter.js:45:38)
at NodeEventGenerator.applySelector (/<removed>/eslint-remote-tester/node_modules/eslint/lib/linter/node-event-generator.js:254:26)
at NodeEventGenerator.applySelectors (/<removed>/eslint-remote-tester/node_modules/eslint/lib/linter/node-event-generator.js:283:22)
at NodeEventGenerator.enterNode (/<removed>/eslint-remote-tester/node_modules/eslint/lib/linter/node-event-generator.js:297:14)
at CodePathAnalyzer.enterNode (/<removed>/eslint-remote-tester/node_modules/eslint/lib/linter/code-path-analysis/code-path-analyzer.js:711:23)
at /<removed>/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 <text>: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 (
<Portal
container={() => {
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 <text>:15
at JSXAttribute (/<removed>/eslint-plugin-react/lib/rules/jsx-handler-names.js:112:52)
Expand All @@ -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"
>
<FormControlLabel
Expand Down
1 change: 1 addition & 0 deletions lib/engine/types.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { Linter } from 'eslint';

export interface LintMessage extends Linter.LintMessage {
path: string;
error?: string;
}

export interface WorkerData {
Expand Down
70 changes: 54 additions & 16 deletions lib/engine/worker-task.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { ESLint, Linter } from 'eslint';

import { LintMessage, WorkerData } from './types';
import config from '@config';
import { getFiles } from '@file-client';
import { getFiles, SourceFile } from '@file-client';

export type WorkerMessage =
| { type: 'START' }
Expand All @@ -22,19 +22,23 @@ export type WorkerMessage =

// Regex used to attempt parsing out rule which caused linter to crash
const RULE_REGEXP = /rules\/(.*?)\.js/;

// Regex used to attempt parsing out line which caused linter to crash
const LINE_REGEX = /while linting <text>:(([0-9]+)?)/;

const SOURCE_WINDOW_SIZE = 10;
const MAX_LINT_TIME_SECONDS = 5;

/**
* Create error message for LintMessage results
*/
export function createErrorMessage(
error: Omit<LintMessage, 'column' | 'line' | 'severity'>
error: Omit<LintMessage, 'column' | 'line' | 'severity'> & { line?: number }
): LintMessage {
return {
line: 0,
...error,
column: 0,
line: 0,
severity: 0,
};
}
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -150,19 +193,14 @@ export default async function workerTask(): Promise<void> {
);
} 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;
}

Expand Down
2 changes: 1 addition & 1 deletion lib/file-client/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
export { getFiles } from './file-client';
export { getFiles, SourceFile } from './file-client';
export {
writeResults,
getResults,
Expand Down
18 changes: 14 additions & 4 deletions lib/file-client/result-templates.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
1 change: 1 addition & 0 deletions lib/file-client/results-writer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ const RESULT_TEMPLATE_CLI = (result: LintMessage) => {
link: `${URL}/${project}/${repository}${postfix}`,
extension,
source: result.source,
error: result.error,
});
};

Expand Down
4 changes: 4 additions & 0 deletions test/integration/integration.ci.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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) {
Expand All @@ -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) {
}
Expand All @@ -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();
Expand Down

0 comments on commit 2737019

Please sign in to comment.