github-mirrors/kibana
1
0
Fork 0
mirror of https://github.com/elastic/kibana.git synced 2025-06-27 18:51:07 -04:00
Code Issues Projects Releases Packages Wiki Activity

chore(NA): prevent disabling eslint protected rules (#137066)

Browse source 
* chore(NA): introduce barebones for no_protected_eslint_disable rule

* chore(NA): includes schema for protected eslint

* chore(NA): more progress on the rule

* chore(NA): first working logic for the rule

* fix(NA): correctly match allowed exclusions

* refact(NA): final refactor to complete rule

* chore(NA): remove non needed additional types

* fix(NA): remove wrongly left custom type

* refact(NA): create parseEslintDisableComment function

* refact(NA): remove option to configure disabled protected rules from configuration

* chore(NA): Update packages/kbn-eslint-plugin-disable/src/helpers/regex.ts

Co-authored-by: Spencer <email@spalger.com>

* refact(NA): use a const instead of function to setup protected rules

* chore(NA): run eslint fix

* refact(NA): removed unused functionality

* add failing test

* refact(NA): rebuild entire comment line on fixing

* refact(NA): joining with a space in between

* chore(NA): improved comparision logic

Co-authored-by: Spencer <email@spalger.com>
Co-authored-by: spalger <spencer@elastic.co>
This commit is contained in:
Tiago Costa 2022-07-27 00:40:01 +01:00 • committed by GitHub
parent 6987598607
commit b1c7efff8a
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
11 changed files with 754 additions and 29 deletions
Download patch file Download diff file Expand all files Collapse all files

1
packages/kbn-eslint-config/.eslintrc.js
View file

@ -248,6 +248,7 @@ module.exports = {
},
]],
'@kbn/disable/no_protected_eslint_disable': 'error',
'@kbn/disable/no_naked_eslint_disable': 'error',
'@kbn/eslint/no_async_promise_body': 'error',
'@kbn/eslint/no_async_foreach': 'error',

1
packages/kbn-eslint-plugin-disable/BUILD.bazel
View file

@ -49,6 +49,7 @@ RUNTIME_DEPS = [
#
# References to NPM packages work the same as RUNTIME_DEPS
TYPES_DEPS = [
"@npm//tslib",
"@npm//@types/eslint",
"@npm//@types/jest",
"@npm//@types/node",

6
packages/kbn-eslint-plugin-disable/README.mdx
View file

@ -10,4 +10,8 @@ tags: ['kibana', 'dev', 'contributor', 'operations', 'eslint', 'disable']
## `@kbn/disable/no_naked_eslint_disable`
Disallows the usage of naked eslint-disable comments without being specific about each rule to disable.
Disallows the usage of naked eslint-disable comments without being specific about each rule to disable.
## `@kbn/disable/no_protected_eslint_disable`
Disallows the eslint-disable comments with specific protected rules that were built to not be disabled. Please contact the Kibana Operations team if you have a justifiable use case for disabling one of them.

11
packages/kbn-eslint-plugin-disable/src/helpers/index.ts Normal file
View file

@ -0,0 +1,11 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/
export * from './protected_rules';
export * from './regex';
export * from './report';

14
packages/kbn-eslint-plugin-disable/src/helpers/protected_rules.ts Normal file
View file

@ -0,0 +1,14 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/
// this configures the protected eslint rules on our codebase that can't be disabled
export const PROTECTED_RULES = new Set([
'@kbn/disable/no_protected_eslint_disable',
'@kbn/disable/no_naked_eslint_disable',
'@kbn/imports/no_unused_imports',
]);

61
packages/kbn-eslint-plugin-disable/src/helpers/regex.ts Normal file
View file

@ -0,0 +1,61 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/
import type Eslint from 'eslint';
const ESLINT_DISABLE_RE = /^eslint-disable(?:-next-line|-line)?(?<rulesBlock>.*)/;
export enum ESLINT_DISABLE_VALUE {
DISABLE = 'eslint-disable',
DISABLE_NEXT_LINE = 'eslint-disable-next-line',
DISABLE_LINE = 'eslint-disable-line',
}
export interface ParsedEslintDisableComment {
type: Eslint.AST.Program['comments'][0]['type'];
range: Eslint.AST.Program['comments'][0]['range'];
loc: Eslint.AST.Program['comments'][0]['loc'];
value: Eslint.AST.Program['comments'][0]['value'];
disableValueType: ESLINT_DISABLE_VALUE;
rules: string[];
}
export function parseEslintDisableComment(
comment: Eslint.AST.Program['comments'][0]
): ParsedEslintDisableComment | undefined {
const commentVal = comment.value.trim();
const nakedESLintRegexResult = commentVal.match(ESLINT_DISABLE_RE);
const rulesBlock = nakedESLintRegexResult?.groups?.rulesBlock;
// no regex match
if (!nakedESLintRegexResult) {
return;
}
const disableValueType = commentVal.includes(ESLINT_DISABLE_VALUE.DISABLE_NEXT_LINE)
? ESLINT_DISABLE_VALUE.DISABLE_NEXT_LINE
: commentVal.includes(ESLINT_DISABLE_VALUE.DISABLE_LINE)
? ESLINT_DISABLE_VALUE.DISABLE_LINE
: ESLINT_DISABLE_VALUE.DISABLE;
const rules = rulesBlock
? rulesBlock
.trim()
.split(',')
.map((r) => r.trim())
: [];
return {
type: comment.type,
range: comment.range,
loc: comment.loc,
value: comment.value,
disableValueType,
rules,
};
}

36
packages/kbn-eslint-plugin-disable/src/helpers/report.ts Normal file
View file

@ -0,0 +1,36 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/
import type Eslint from 'eslint';
import { ESLINT_DISABLE_VALUE, ParsedEslintDisableComment } from './regex';
export function getReportLocFromComment(
comment: ParsedEslintDisableComment
): Eslint.AST.SourceLocation | undefined {
const cStart = comment?.loc?.start;
const cEnd = comment?.loc?.end;
const cStartLine = comment?.loc?.start?.line;
// start or end loc is undefined, exit early
if (cStart === undefined || cEnd === undefined || cStartLine === undefined) {
return;
}
const disableStartsOnNextLine =
comment.disableValueType === ESLINT_DISABLE_VALUE.DISABLE_NEXT_LINE;
const disableStartsInline = comment.disableValueType === ESLINT_DISABLE_VALUE.DISABLE_LINE;
const cStartColumn = comment?.loc?.start?.column ?? 0;
return disableStartsOnNextLine
? { start: cStart, end: cEnd }
: {
// At this point we could have eslint-disable block or an eslint-disable-line.
// If we have an inline disable we need to report the column as -1 in order to get the report
start: { line: cStartLine, column: disableStartsInline ? -1 : cStartColumn - 1 },
end: cEnd,
};
}

2
packages/kbn-eslint-plugin-disable/src/index.ts
View file

@ -7,6 +7,7 @@
*/
import { NoNakedESLintDisableRule } from './rules/no_naked_eslint_disable';
import { NoProtectedESLintDisableRule } from './rules/no_protected_eslint_disable';
/**
* Custom ESLint rules, add `'@kbn/eslint-plugin-disable'` to your eslint config to use them
@ -14,4 +15,5 @@ import { NoNakedESLintDisableRule } from './rules/no_naked_eslint_disable';
*/
export const rules = {
no_naked_eslint_disable: NoNakedESLintDisableRule,
no_protected_eslint_disable: NoProtectedESLintDisableRule,
};

38
packages/kbn-eslint-plugin-disable/src/rules/no_naked_eslint_disable.ts
View file

@ -7,6 +7,7 @@
*/
import Eslint from 'eslint';
import { getReportLocFromComment, parseEslintDisableComment } from '../helpers';
export const NAKED_DISABLE_MSG_ID = 'no-naked-eslint-disable';
const messages = {
@ -24,57 +25,38 @@ const meta: Eslint.Rule.RuleMetaData = {
messages,
};
const ESLINT_DISABLE_RE =
/^eslint-disable(?:-next-line|-line)?(?<ruleName>$|(?:\s+(?:@(?:[\w-]+\/){1,2})?[\w-]+)?)/;
const create = (context: Eslint.Rule.RuleContext): Eslint.Rule.RuleListener => {
return {
Program(node) {
const nodeComments = node.comments || [];
nodeComments.forEach((comment) => {
const commentVal = comment.value.trim();
const nakedESLintRegexResult = commentVal.match(ESLINT_DISABLE_RE);
const ruleName = nakedESLintRegexResult?.groups?.ruleName;
// get parsedEslintDisable from comment
const parsedEslintDisable = parseEslintDisableComment(comment);
// no regex match, exit early
if (!nakedESLintRegexResult) {
if (!parsedEslintDisable) {
return;
}
// we have a rule name, exit early
if (ruleName) {
// we have a rule name so we can exit early
if (parsedEslintDisable.rules.length > 0) {
return;
}
const cStart = comment?.loc?.start;
const cEnd = comment?.loc?.end;
const cStartLine = comment?.loc?.start?.line;
// start or end loc is undefined, exit early
if (cStart === undefined || cEnd === undefined || cStartLine === undefined) {
// collect position to report
const reportLoc = getReportLocFromComment(parsedEslintDisable);
if (!reportLoc) {
return;
}
const disableStartsOnNextLine = comment.value.includes('disable-next-line');
const disableStartsInline = comment.value.includes('disable-line');
const cStartColumn = comment?.loc?.start?.column ?? 0;
const reportLoc = disableStartsOnNextLine
? { start: cStart, end: cEnd }
: {
// At this point we could have eslint-disable block or an eslint-disable-line.
// If we have an inline disable we need to report the column as -1 in order to get the report
start: { line: cStartLine, column: disableStartsInline ? -1 : cStartColumn - 1 },
end: cEnd,
};
// At this point we have a regex match, no rule name and a valid loc so lets report here
context.report({
node,
loc: reportLoc,
messageId: NAKED_DISABLE_MSG_ID,
fix(fixer) {
return fixer.removeRange(comment.range as Eslint.AST.Range);
return fixer.removeRange(parsedEslintDisable.range as Eslint.AST.Range);
},
});
});

514
packages/kbn-eslint-plugin-disable/src/rules/no_protected_eslint_disable.test.ts Normal file
View file

@ -0,0 +1,514 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/
import dedent from 'dedent';
import { RuleTester } from 'eslint';
import {
NoProtectedESLintDisableRule,
PROTECTED_DISABLE_MSG_ID,
} from './no_protected_eslint_disable';
jest.mock('../helpers/protected_rules', () => {
return {
PROTECTED_RULES: new Set(['@kbn/disable/no_protected_eslint_disable', 'no-console']),
};
});
const tsTester = [
'@typescript-eslint/parser',
new RuleTester({
parser: require.resolve('@typescript-eslint/parser'),
parserOptions: {
sourceType: 'module',
ecmaVersion: 2018,
ecmaFeatures: {
jsx: true,
},
},
}),
] as const;
const babelTester = [
'@babel/eslint-parser',
new RuleTester({
parser: require.resolve('@babel/eslint-parser'),
parserOptions: {
sourceType: 'module',
ecmaVersion: 2018,
requireConfigFile: false,
babelOptions: {
presets: ['@kbn/babel-preset/node_preset'],
},
},
}),
] as const;
for (const [name, tester] of [tsTester, babelTester]) {
describe(name, () => {
tester.run('@kbn/disable/no_protected_eslint_disable', NoProtectedESLintDisableRule, {
valid: [
{
filename: 'foo.ts',
code: dedent`
// eslint-disable no-var
`,
},
{
filename: 'foo.ts',
code: dedent`
// eslint-disable-next-line no-use-before-define
`,
},
{
filename: 'foo.ts',
code: dedent`
// eslint-disable-line no-use-before-define
`,
},
{
filename: 'foo.ts',
code: dedent`
/* eslint-disable no-var */
`,
},
{
filename: 'foo.ts',
code: dedent`
/* eslint-disable no-var, no-control-regex*/
`,
},
{
filename: 'foo.ts',
code: dedent`
alert('foo'); // eslint-disable-line no-alert
`,
},
{
filename: 'foo.ts',
code: dedent`
const foo = 'foo';
let bar = 'ba';
/* eslint-disable no-alert */
alert(foo);
/* eslint-enable no-alert */
bar += 'r';
`,
},
{
filename: 'foo.ts',
code: dedent`
const foo = 'foo';
let bar = 'ba';
/* eslint-disable-next-line no-alert */
alert(foo);
bar += 'r';
`,
},
{
filename: 'foo.ts',
code: dedent`
const foo = 'foo';
let bar = 'ba';
alert(foo);/* eslint-disable-line no-alert */
bar += 'r';
`,
},
],
invalid: [
{
filename: 'foo.ts',
code: dedent`
/* eslint-disable no-var,no-console */
const a = 1;
`,
errors: [
{
line: 1,
messageId: PROTECTED_DISABLE_MSG_ID,
data: {
disabledRuleName: 'no-console',
},
},
],
output: dedent`
/* eslint-disable no-var */
const a = 1;
`,
},
{
filename: 'foo.ts',
code: dedent`
/* eslint-disable no-var,no-console*/
const a = 1;
`,
errors: [
{
line: 1,
messageId: PROTECTED_DISABLE_MSG_ID,
data: {
disabledRuleName: 'no-console',
},
},
],
output: dedent`
/* eslint-disable no-var */
const a = 1;
`,
},
{
filename: 'foo.ts',
code: dedent`
/*eslint-disable no-var,no-console*/
const a = 1;
`,
errors: [
{
line: 1,
messageId: PROTECTED_DISABLE_MSG_ID,
data: {
disabledRuleName: 'no-console',
},
},
],
output: dedent`
/* eslint-disable no-var */
const a = 1;
`,
},
{
filename: 'foo.ts',
code: dedent`
// eslint-disable no-var,no-console
const a = 1;
`,
errors: [
{
line: 1,
messageId: PROTECTED_DISABLE_MSG_ID,
data: {
disabledRuleName: 'no-console',
},
},
],
output: dedent`
// eslint-disable no-var
const a = 1;
`,
},
{
filename: 'foo.ts',
code: dedent`
//eslint-disable no-var,no-console
const a = 1;
`,
errors: [
{
line: 1,
messageId: PROTECTED_DISABLE_MSG_ID,
data: {
disabledRuleName: 'no-console',
},
},
],
output: dedent`
// eslint-disable no-var
const a = 1;
`,
},
{
filename: 'foo.ts',
code: dedent`
/* eslint-disable no-var,@kbn/disable/no_protected_eslint_disable */
const a = 1;
`,
errors: [
{
line: 1,
messageId: PROTECTED_DISABLE_MSG_ID,
data: {
disabledRuleName: '@kbn/disable/no_protected_eslint_disable',
},
},
],
output: dedent`
/* eslint-disable no-var */
const a = 1;
`,
},
{
filename: 'foo.ts',
code: dedent`
/* eslint-disable no-var, @kbn/disable/no_protected_eslint_disable */
const a = 1;
`,
errors: [
{
line: 1,
messageId: PROTECTED_DISABLE_MSG_ID,
data: {
disabledRuleName: '@kbn/disable/no_protected_eslint_disable',
},
},
],
output: dedent`
/* eslint-disable no-var */
const a = 1;
`,
},
{
filename: 'foo.ts',
code: dedent`
/* eslint-disable @kbn/disable/no_protected_eslint_disable, no-var */
const a = 1;
`,
errors: [
{
line: 1,
messageId: PROTECTED_DISABLE_MSG_ID,
data: {
disabledRuleName: '@kbn/disable/no_protected_eslint_disable',
},
},
],
output: dedent`
/* eslint-disable no-var */
const a = 1;
`,
},
{
filename: 'foo.ts',
code: dedent`
/* eslint-disable prefer-const, @kbn/disable/no_protected_eslint_disable, no-var */
const a = 1;
`,
errors: [
{
line: 1,
messageId: PROTECTED_DISABLE_MSG_ID,
data: {
disabledRuleName: '@kbn/disable/no_protected_eslint_disable',
},
},
],
output: dedent`
/* eslint-disable prefer-const, no-var */
const a = 1;
`,
},
{
filename: 'foo.ts',
code: dedent`
/* eslint-disable no-var, @kbn/disable/no_protected_eslint_disable, prefer-const */
const a = 1;
`,
errors: [
{
line: 1,
messageId: PROTECTED_DISABLE_MSG_ID,
data: {
disabledRuleName: '@kbn/disable/no_protected_eslint_disable',
},
},
],
output: dedent`
/* eslint-disable no-var, prefer-const */
const a = 1;
`,
},
// generic invalid tests for disable comments
{
filename: 'foo.ts',
code: dedent`
/* eslint-disable no-console */
const a = 1;
`,
errors: [
{
line: 1,
messageId: PROTECTED_DISABLE_MSG_ID,
data: {
disabledRuleName: 'no-console',
},
},
],
output: '\nconst a = 1;',
},
{
filename: 'foo.ts',
code: dedent`
// eslint-disable-next-line no-console
const a = 1;
`,
errors: [
{
line: 1,
messageId: PROTECTED_DISABLE_MSG_ID,
data: {
disabledRuleName: 'no-console',
},
},
],
output: `\nconst a = 1;`,
},
{
filename: 'foo.ts',
code: dedent`
/* eslint-disable no-console*/
`,
errors: [
{
line: 1,
messageId: PROTECTED_DISABLE_MSG_ID,
data: {
disabledRuleName: 'no-console',
},
},
],
output: '',
},
{
filename: 'foo.ts',
code: dedent`
// eslint-disable-next-line no-console
`,
errors: [
{
line: 1,
messageId: PROTECTED_DISABLE_MSG_ID,
data: {
disabledRuleName: 'no-console',
},
},
],
output: '',
},
{
filename: 'foo.ts',
code: dedent`
alert('foo');// eslint-disable-line no-console
`,
errors: [
{
line: 1,
messageId: PROTECTED_DISABLE_MSG_ID,
data: {
disabledRuleName: 'no-console',
},
},
],
output: `alert('foo');`,
},
{
filename: 'foo.ts',
code: dedent`
const foo = 'foo';
let bar = 'ba';
/* eslint-disable no-console */
alert(foo);
/* eslint-enable */
bar += 'r';
`,
errors: [
{
line: 3,
messageId: PROTECTED_DISABLE_MSG_ID,
data: {
disabledRuleName: 'no-console',
},
},
],
output: dedent`
const foo = 'foo';
let bar = 'ba';
alert(foo);
/* eslint-enable */
bar += 'r';
`,
},
{
filename: 'foo.ts',
code: dedent`
const foo = 'foo';
let bar = 'ba';
/* eslint-disable no-console */
alert(foo);
bar += 'r';
`,
errors: [
{
line: 3,
messageId: PROTECTED_DISABLE_MSG_ID,
data: {
disabledRuleName: 'no-console',
},
},
],
output: dedent`
const foo = 'foo';
let bar = 'ba';
alert(foo);
bar += 'r';
`,
},
{
filename: 'foo.ts',
code: dedent`
const foo = 'foo';
let bar = 'ba';
/* eslint-disable-next-line no-console */
alert(foo);
bar += 'r';
`,
errors: [
{
line: 3,
messageId: PROTECTED_DISABLE_MSG_ID,
data: {
disabledRuleName: 'no-console',
},
},
],
output: dedent`
const foo = 'foo';
let bar = 'ba';
alert(foo);
bar += 'r';
`,
},
{
filename: 'foo.ts',
code: dedent`
const foo = 'foo';
let bar = 'ba';
alert(foo);/* eslint-disable-line no-console */
bar += 'r';
`,
errors: [
{
line: 3,
messageId: PROTECTED_DISABLE_MSG_ID,
data: {
disabledRuleName: 'no-console',
},
},
],
output: dedent`
const foo = 'foo';
let bar = 'ba';
alert(foo);
bar += 'r';
`,
},
],
});
});
}

99
packages/kbn-eslint-plugin-disable/src/rules/no_protected_eslint_disable.ts Normal file
View file

@ -0,0 +1,99 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/
import Eslint from 'eslint';
import { PROTECTED_RULES, getReportLocFromComment, parseEslintDisableComment } from '../helpers';
export const PROTECTED_DISABLE_MSG_ID = 'no-protected-eslint-disable';
const messages = {
[PROTECTED_DISABLE_MSG_ID]:
"The rule '{{ disabledRuleName }}' is protected and disabling it is not allowed. Please remove it from the statement.",
};
const meta: Eslint.Rule.RuleMetaData = {
type: 'problem',
fixable: 'code',
docs: {
description: 'Prevents the disabling of protected rules within eslint-disable* comments.',
},
messages,
};
const create = (context: Eslint.Rule.RuleContext): Eslint.Rule.RuleListener => {
return {
Program(node) {
const nodeComments = node.comments || [];
nodeComments.forEach((comment) => {
// get parsedEslintDisable from comment
const parsedEslintDisable = parseEslintDisableComment(comment);
// no regex match, exit early
if (!parsedEslintDisable) {
return;
}
// we do not have a rule block, exit early
if (parsedEslintDisable.rules.length === 0) {
return;
}
const disabledRules = parsedEslintDisable.rules;
const disabledProtectedRule = disabledRules.find((r) => PROTECTED_RULES.has(r));
// no protected rule was disabled, exit early
if (!disabledProtectedRule) {
return;
}
// at this point we'll collect the disabled rule position to report
const reportLoc = getReportLocFromComment(parsedEslintDisable);
if (!reportLoc) {
return;
}
// At this point we have a regex match, no rule name and a valid loc so lets report here
context.report({
node,
loc: reportLoc,
messageId: PROTECTED_DISABLE_MSG_ID,
data: {
disabledRuleName: disabledProtectedRule,
},
fix(fixer) {
// if we only have a single disabled rule and that is protected, we can remove the entire comment
if (disabledRules.length === 1) {
return fixer.removeRange(parsedEslintDisable.range as Eslint.AST.Range);
}
// it's impossible to fix as we don't have a range
if (!parsedEslintDisable.range) {
return null;
}
const remainingRules = disabledRules.filter((rule) => !PROTECTED_RULES.has(rule));
const fixedComment = ` ${parsedEslintDisable.disableValueType} ${remainingRules.join(
', '
)}${parsedEslintDisable.type === 'Block' ? ' ' : ''}`;
const rangeToFix: Eslint.AST.Range =
parsedEslintDisable.type === 'Line'
? [parsedEslintDisable.range[0] + 2, parsedEslintDisable.range[1]]
: [parsedEslintDisable.range[0] + 2, parsedEslintDisable.range[1] - 2];
return fixer.replaceTextRange(rangeToFix, fixedComment);
},
});
});
},
};
};
export const NoProtectedESLintDisableRule: Eslint.Rule.RuleModule = {
meta,
create,
};