Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade ESLint to 7.5.0 and typescript-eslint to 3.7.1 #2038

Merged
merged 7 commits into from Jul 31, 2020
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
22 changes: 13 additions & 9 deletions eslint-bridge/package.json
Expand Up @@ -24,13 +24,14 @@
"node": ">=6"
},
"devDependencies": {
"@types/eslint": "4.16.3",
"@types/eslint": "7.2.0",
"@types/eslint-scope": "3.7.0",
"@types/esprima": "4.0.2",
"@types/estree": "0.0.39",
"@types/estree": "0.0.45",
"@types/express": "4.16.0",
"@types/jest": "23.3.2",
"@types/node": "13.1.4",
"@types/semver": "7.3.1",
"fs-extra": "7.0.0",
"jest": "24.9.0",
"jest-sonar-reporter": "1.3.0",
Expand All @@ -40,18 +41,19 @@
"typescript": "3.9.5"
},
"dependencies": {
"@typescript-eslint/eslint-plugin": "2.23.0",
"@typescript-eslint/experimental-utils": "2.23.0",
"@typescript-eslint/parser": "2.23.0",
"@typescript-eslint/eslint-plugin": "3.7.1",
"@typescript-eslint/experimental-utils": "3.7.1",
"@typescript-eslint/parser": "3.7.1",
"babel-eslint": "10.1.0",
"body-parser": "1.18.3",
"builtin-modules": "3.1.0",
"eslint": "5.16.0",
"eslint": "7.5.0",
"eslint-plugin-chai-friendly": "0.5.0",
"eslint-plugin-sonarjs": "0.5.0",
"espree": "5.0.1",
"espree": "7.2.0",
"express": "4.16.3",
"run-node": "1.0.0",
"semver": "7.3.2",
"vue-eslint-parser": "6.0.4"
},
"bundledDependencies": [
Expand All @@ -64,9 +66,11 @@
"eslint",
"eslint-plugin-chai-friendly",
"eslint-plugin-sonarjs",
"espree",
"express",
"vue-eslint-parser",
"run-node"
"run-node",
"semver",
"vue-eslint-parser"
],
"prettier": {
"printWidth": 100,
Expand Down
10 changes: 6 additions & 4 deletions eslint-bridge/src/parser.ts
Expand Up @@ -22,12 +22,14 @@ import * as babel from 'babel-eslint';
import { Linter, SourceCode } from 'eslint';
import { ParsingError } from './analyzer';
import * as VueJS from 'vue-eslint-parser';
import * as semver from 'semver';

// this value is taken from typescript-estree
// still we might consider extending this range
// if everything which we need is working on older/newer versions
const TYPESCRIPT_MINIMUM_VERSION = '3.2.1';
const TYPESCRIPT_MAXIMUM_VERSION = '3.8.0';
const TYPESCRIPT_MINIMUM_VERSION = '3.3.1';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should update documentation.md

// next released version is 4.0.0, we need version which is above current 3.9.x and below 4.0.0
const TYPESCRIPT_MAXIMUM_VERSION = '3.10.0';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

where does this version come from? I see <4.0.0 for typescript-eslint

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


export const PARSER_CONFIG_MODULE: Linter.ParserOptions = {
tokens: true,
Expand Down Expand Up @@ -122,13 +124,13 @@ export function resetReportedNewerTypeScriptVersion() {

// exported for testing
export function checkTypeScriptVersionCompatibility(currentVersion: string) {
if (currentVersion >= TYPESCRIPT_MAXIMUM_VERSION && !reportedNewerTypeScriptVersion) {
if (semver.gt(currentVersion, TYPESCRIPT_MAXIMUM_VERSION) && !reportedNewerTypeScriptVersion) {
reportedNewerTypeScriptVersion = true;
console.log(
`WARN You are using version of TypeScript ${currentVersion} which is not officially supported; ` +
`supported versions >=${TYPESCRIPT_MINIMUM_VERSION} <${TYPESCRIPT_MAXIMUM_VERSION}`,
);
} else if (currentVersion < TYPESCRIPT_MINIMUM_VERSION) {
} else if (semver.lt(currentVersion, TYPESCRIPT_MINIMUM_VERSION)) {
throw {
message: `You are using version of TypeScript ${currentVersion} which is not supported; supported versions >=${TYPESCRIPT_MINIMUM_VERSION}`,
};
Expand Down
12 changes: 1 addition & 11 deletions eslint-bridge/src/rules/cyclomatic-complexity.ts
Expand Up @@ -85,7 +85,7 @@ function raiseOnUnauthorizedComplexity(
if (complexity > threshold) {
context.report({
message: toEncodedMessage(complexity, threshold, tokens),
loc: toPrimaryLocation(node, context),
loc: getMainFunctionTokenLocation(node, parent, context),
});
}
}
Expand All @@ -103,16 +103,6 @@ function toEncodedMessage(
return JSON.stringify(encodedMessage);
}

function toPrimaryLocation(node: estree.Node, context: Rule.RuleContext): IssueLocation {
const func = node as FunctionNodeType;
return {
line: func.loc!.start.line,
column: func.loc!.start.column,
endLine: context.getSourceCode().getTokenBefore(func.body)!.loc.end.line,
endColumn: context.getSourceCode().getTokenBefore(func.body)!.loc.end.column,
};
}

function toSecondaryLocation(token: ComplexityToken): IssueLocation {
return {
line: token.loc.start.line,
Expand Down
4 changes: 2 additions & 2 deletions eslint-bridge/src/rules/no-duplicate-in-composite.ts
Expand Up @@ -36,15 +36,15 @@ export const rule: Rule.RuleModule = {

create(context: Rule.RuleContext) {
return {
'TSUnionType, TSIntersectionType': function (node: estree.Node) {
'TSUnionType, TSIntersectionType'(node: estree.Node) {
const sourceCode = context.getSourceCode();
const compositeType = (node as unknown) as
| TSESTree.TSUnionType
| TSESTree.TSIntersectionType;
const groupedTypes: Map<string, Array<TSESTree.Node>> = new Map();

compositeType.types.forEach(typescriptType => {
const nodeValue = sourceCode.getText(typescriptType as estree.Node);
const nodeValue = sourceCode.getText((typescriptType as unknown) as estree.Node);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why did it compile before? without as unknown

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

something has changed in the type definitions that they don't overlap anymore, I didn't dig deeper

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think some types are out of sync between TSESTree and estree

const nodesWithGivenType = groupedTypes.get(nodeValue);
const nodeType = typescriptType as TSESTree.Node;
if (!nodesWithGivenType) {
Expand Down
6 changes: 5 additions & 1 deletion eslint-bridge/src/rules/no-globals-shadowing.ts
Expand Up @@ -90,7 +90,11 @@ function reportBadUsage(
break;
case 'ObjectPattern':
node.properties.forEach(prop => {
reportBadUsage(prop.value, buildMessage, context);
if (prop.type === 'Property') {
reportBadUsage(prop.value, buildMessage, context);
} else {
reportBadUsage(prop.argument, buildMessage, context);
}
});
break;
case 'ArrayPattern':
Expand Down
2 changes: 2 additions & 0 deletions eslint-bridge/src/rules/no-unenclosed-multiline-block.ts
Expand Up @@ -46,6 +46,8 @@ export const rule: Rule.RuleModule = {
Program: (node: estree.Node) => checkStatements((node as estree.Program).body, context),
BlockStatement: (node: estree.Node) =>
checkStatements((node as estree.BlockStatement).body, context),
TSModuleBlock: (node: estree.Node) =>
checkStatements(((node as unknown) as TSESTree.TSModuleBlock).body as Statement[], context),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in fact this is a fix for FN. Do you think it deserves a separate ticket in backlog?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is minor improvement, let's keep it here

};
},
};
Expand Down
2 changes: 1 addition & 1 deletion eslint-bridge/src/rules/no-useless-intersection.ts
Expand Up @@ -50,7 +50,7 @@ export const rule: Rule.RuleModule = {
if (isTypeWithoutMembers(tp, ts)) {
context.report({
message: 'Remove this type without members or change this type intersection.',
node: typeNode as estree.Node,
node: (typeNode as unknown) as estree.Node,
});
}
});
Expand Down
2 changes: 1 addition & 1 deletion eslint-bridge/src/rules/os-command.ts
Expand Up @@ -91,7 +91,7 @@ function containsShellOption(otherArguments: Argument[]) {
return otherArguments.some(
arg =>
arg.type === 'ObjectExpression' &&
arg.properties.some(
(arg.properties.filter(v => v.type === 'Property') as estree.Property[]).some(
({ key, value }) =>
isIdentifier(key, 'shell') && value.type === 'Literal' && value.value === true,
),
Expand Down
2 changes: 1 addition & 1 deletion eslint-bridge/src/rules/prefer-type-guard.ts
Expand Up @@ -118,7 +118,7 @@ function getCastTupleFromMemberExpression(
if (node.type === 'MemberExpression') {
const object = node.object as TSESTree.Node;
if (object.type === 'TSAsExpression' || object.type === 'TSTypeAssertion') {
return [object.expression as estree.Node, object.typeAnnotation as estree.Node];
return [object.expression as estree.Node, (object.typeAnnotation as unknown) as estree.Node];
}
}
return undefined;
Expand Down
6 changes: 4 additions & 2 deletions eslint-bridge/src/rules/shorthand-property-grouping.ts
Expand Up @@ -45,7 +45,7 @@ export const rule: Rule.RuleModule = {
const secondaryMessages = [];

for (let i = begin; i < end; i++) {
const prop = properties[i];
const prop = properties[i] as estree.Property;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and here, what about SpreadElement?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here it is guarded by condition on line 70

if (prop.shorthand) {
secondaryNodes.push(prop);
secondaryMessages.push(`Move to ${positionMessage}`);
Expand All @@ -70,7 +70,9 @@ export const rule: Rule.RuleModule = {
if (objectExpressionProperties.some(p => p.type !== 'Property')) {
return;
}
const isShorthandPropertyList = objectExpressionProperties.map(p => p.shorthand);
const isShorthandPropertyList = objectExpressionProperties.map(
p => (p as estree.Property).shorthand,
);
const shorthandPropertiesNumber = isShorthandPropertyList.filter(b => b).length;

const numberOfShorthandAtBeginning = getNumberOfTrueAtBeginning(isShorthandPropertyList);
Expand Down
4 changes: 3 additions & 1 deletion eslint-bridge/src/rules/use-type-alias.ts
Expand Up @@ -58,7 +58,9 @@ export const rule: Rule.RuleModule = {
const composite = (node as unknown) as TSESTree.TSUnionType | TSESTree.TSIntersectionType;
if (composite.types.length > TYPE_THRESHOLD) {
const text = composite.types
.map(typeNode => context.getSourceCode().getText(typeNode as estree.Node))
.map(typeNode =>
context.getSourceCode().getText((typeNode as unknown) as estree.Node),
)
.sort((a, b) => a.localeCompare(b))
.join('|');
let occurrences = usage.get(text);
Expand Down
3 changes: 2 additions & 1 deletion eslint-bridge/src/utils/decorators.ts
Expand Up @@ -37,7 +37,8 @@ export function interceptReport(
onReport: (context: Rule.RuleContext, reportDescriptor: Rule.ReportDescriptor) => void,
): Rule.RuleModule {
return {
meta: rule.meta,
// meta should be defined only when it's defined on original rule, otherwise RuleTester will fail
...(!!rule.meta && { meta: rule.meta }),
create(originalContext: Rule.RuleContext) {
const interceptingContext: Rule.RuleContext = {
id: originalContext.id,
Expand Down
7 changes: 0 additions & 7 deletions eslint-bridge/tests/RuleTesterTs.ts
Expand Up @@ -62,13 +62,6 @@ class RuleTesterTs extends RuleTester {
invalid?: RuleTester.InvalidTestCase[];
},
): void {
if (this.expectNoIssuesWithoutTypes) {
this.ruleTesterNoTsConfig.run(`${name}[noTsConfig]`, rule, {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove this.ruleTesterNoTsConfig

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@saberduck not handled

valid: tests.invalid,
invalid: [],
});
}

tests.valid.forEach(test => {
if (!test.filename) {
test.filename = placeHolderFilePath;
Expand Down
8 changes: 4 additions & 4 deletions eslint-bridge/tests/parser.test.ts
Expand Up @@ -187,19 +187,19 @@ describe('parseTypeScriptSourceFile', () => {
expect(parsingException).toBeDefined;
expect(parsingException).toEqual({
message:
'You are using version of TypeScript 1.2.3 which is not supported; supported versions >=3.2.1',
'You are using version of TypeScript 1.2.3 which is not supported; supported versions >=3.3.1',
});
});

it('should log a warning with TypeScript version above maximum expected', () => {
console.log = jest.fn();
resetReportedNewerTypeScriptVersion();
checkTypeScriptVersionCompatibility('3.8.5');
checkTypeScriptVersionCompatibility('5.0.0');
expect(console.log).toHaveBeenCalledWith(
'WARN You are using version of TypeScript 3.8.5 which is not officially supported; supported versions >=3.2.1 <3.8.0',
'WARN You are using version of TypeScript 5.0.0 which is not officially supported; supported versions >=3.3.1 <3.10.0',
);
console.log = jest.fn();
checkTypeScriptVersionCompatibility('3.8.5');
checkTypeScriptVersionCompatibility('5.0.0');
// should log only once
expect(console.log).not.toHaveBeenCalled();

Expand Down
2 changes: 1 addition & 1 deletion eslint-bridge/tests/rules/bool-param-default.test.ts
Expand Up @@ -83,7 +83,7 @@ ruleTester.run('Optional boolean parameters should have default value', rule, {
errors: 1,
},
{
code: `let f = (b?: boolean) b;`,
code: `let f = (b?: boolean) => b;`,
errors: 1,
},
{
Expand Down