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

feat: add JSXSpreadChild and tool to build keys out of AST definitions #36

Merged
merged 14 commits into from
Mar 29, 2022
Merged
Show file tree
Hide file tree
Changes from 11 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
12 changes: 12 additions & 0 deletions lib/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,18 @@ import KEYS from "./visitor-keys.js";
// List to ignore keys.
const KEY_BLACKLIST = new Set([
"parent",

// "type",

// `BaseNodeWithoutComments`
// "range",

// `SourceLocation`
// "loc",

// `Comment`
// "comments",
// "innerComments",
Copy link
Member

Choose a reason for hiding this comment

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

Can we remove these lines? We usually avoid having commented-out code in the codebase unless it is associated with a TODO action.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure. I had thought these were going to be removed in a future version, so should I add a comment to do so? File an issue? Is there any particular approach for planning ahead with to-dos to occur before breaking changes?

eslint-plugin-unicorn has a truly excellent expiring-todo-comments rule which can be tied to version or dependency changes which will cause linting to fail unless applied. I might recommend unicorn for eslint-config-eslint anyways.

Copy link
Member

Choose a reason for hiding this comment

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

I think it would be best to discuss this in an issue first.

Copy link
Member

Choose a reason for hiding this comment

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

Sure. I had thought these were going to be removed in a future version, so should I add a comment to do so? File an issue? Is there any particular approach for planning ahead with to-dos to occur before breaking changes?

We are using GitHub issues to track planned features and bug fixes. We are rarely using TODO comments, mostly for code parts that we think should be refactored.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, thanks for the feedback. Added #38 and removed the comments.

And FWIW, re: Unicorn, I filed eslint/eslint#15731 . Even when not heavily using to-dos in code, having those conditional to-dos can really help ensure that version-dependent to-dos don't get lost.

"leadingComments",
"trailingComments"
]);
Expand Down
73 changes: 38 additions & 35 deletions lib/visitor-keys.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,6 @@
* @type {VisitorKeys}
*/
const KEYS = {
AssignmentExpression: [
"left",
"right"
],
AssignmentPattern: [
"left",
"right"
],
ArrayExpression: [
"elements"
],
Expand All @@ -24,16 +16,24 @@ const KEYS = {
"params",
"body"
],
AssignmentExpression: [
"left",
"right"
],
AssignmentPattern: [
"left",
"right"
],
AwaitExpression: [
"argument"
],
BlockStatement: [
"body"
],
BinaryExpression: [
"left",
"right"
],
BlockStatement: [
"body"
],
BreakStatement: [
"label"
],
Expand Down Expand Up @@ -75,6 +75,12 @@ const KEYS = {
"test"
],
EmptyStatement: [],
ExperimentalRestProperty: [
"argument"
],
ExperimentalSpreadProperty: [
"argument"
],
ExportAllDeclaration: [
"exported",
"source"
Expand All @@ -94,18 +100,6 @@ const KEYS = {
ExpressionStatement: [
"expression"
],
ExperimentalRestProperty: [
"argument"
],
ExperimentalSpreadProperty: [
"argument"
],
brettz9 marked this conversation as resolved.
Show resolved Hide resolved
ForStatement: [
"init",
"test",
"update",
"body"
],
ForInStatement: [
"left",
"right",
Expand All @@ -116,6 +110,12 @@ const KEYS = {
"right",
"body"
],
ForStatement: [
"init",
"test",
"update",
"body"
],
FunctionDeclaration: [
"id",
"params",
Expand Down Expand Up @@ -156,6 +156,7 @@ const KEYS = {
JSXClosingElement: [
"name"
],
JSXClosingFragment: [],
JSXElement: [
"openingElement",
"children",
Expand All @@ -165,6 +166,11 @@ const KEYS = {
JSXExpressionContainer: [
"expression"
],
JSXFragment: [
"openingFragment",
"children",
"closingFragment"
],
JSXIdentifier: [],
JSXMemberExpression: [
"object",
Expand All @@ -178,22 +184,19 @@ const KEYS = {
"name",
"attributes"
],
JSXOpeningFragment: [],
JSXSpreadAttribute: [
"argument"
],
JSXText: [],
JSXFragment: [
"openingFragment",
"children",
"closingFragment"
JSXSpreadChild: [
"expression"
],
brettz9 marked this conversation as resolved.
Show resolved Hide resolved
JSXClosingFragment: [],
JSXOpeningFragment: [],
Literal: [],
JSXText: [],
LabeledStatement: [
"label",
"body"
],
Literal: [],
LogicalExpression: [
"left",
"right"
Expand Down Expand Up @@ -248,14 +251,14 @@ const KEYS = {
"body"
],
Super: [],
SwitchStatement: [
"discriminant",
"cases"
],
SwitchCase: [
"test",
"consequent"
],
SwitchStatement: [
"discriminant",
"cases"
],
TaggedTemplateExpression: [
"tag",
"quasi"
Expand Down
15 changes: 11 additions & 4 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -25,24 +25,31 @@
"node": "^12.22.0 || ^14.17.0 || >=16.0.0"
},
"devDependencies": {
"c8": "^7.7.3",
"@types/estree": "^0.0.51",
"@types/estree-jsx": "^0.0.1",
"@typescript-eslint/parser": "^5.14.0",
"c8": "^7.11.0",
"chai": "^4.3.6",
"eslint": "^7.29.0",
"eslint-config-eslint": "^7.0.0",
"eslint-plugin-jsdoc": "^35.4.0",
"eslint-plugin-node": "^11.1.0",
"eslint-release": "^3.2.0",
"mocha": "^9.0.1",
"esquery": "^1.4.0",
"json-diff": "^0.7.3",
"mocha": "^9.2.1",
"opener": "^1.5.2",
"rollup": "^2.52.1",
"rollup": "^2.70.0",
"tsd": "^0.19.1",
"typescript": "^4.5.5"
"typescript": "^4.6.2"
},
"scripts": {
"prepare": "npm run build",
"build": "rollup -c && npm run tsc",
"lint": "eslint .",
"tsc": "tsc",
"tsd": "tsd",
"build-keys": "node tools/build-keys-from-ts",
"test": "mocha tests/lib/**/*.cjs && c8 mocha tests/lib/**/*.js && npm run tsd",
"coverage": "c8 report --reporter lcov && opener coverage/lcov-report/index.html",
"generate-release": "eslint-generate-release",
Expand Down
3 changes: 3 additions & 0 deletions tests/lib/fixtures/bad-extends-type-reference.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export interface Something extends BadSomething {
type: "Something";
}
5 changes: 5 additions & 0 deletions tests/lib/fixtures/bad-type-parameters.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
export interface Statement {}

export interface StaticBlock extends BadTypeParam<Statement, 'type'> {
type: "StaticBlock";
}
3 changes: 3 additions & 0 deletions tests/lib/fixtures/bad-type-reference.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export interface StaticBlock extends Omit<SomeUnknownStatement, 'type'> {
type: "StaticBlock";
}
8 changes: 8 additions & 0 deletions tests/lib/fixtures/bad-type-value.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
interface BadExpression {
type: undefined;
}

export interface NewFangledExpression {
type: "NewFangledExpression";
right: BadExpression;
}
4 changes: 4 additions & 0 deletions tests/lib/fixtures/bad-type.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
export interface SomeExpression {
type: "SomeExpression";
someProperty: any;
}
4 changes: 4 additions & 0 deletions tests/lib/fixtures/new-keys-bad.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
export interface NewFangledExpression {
type: "NewFangledExpression";
right: BadExpression;
}
20 changes: 20 additions & 0 deletions tests/lib/fixtures/new-keys-on-old-order-switched.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
export type AssignmentOperator = "=";
interface Pattern {
type: "Pattern"
};
interface MemberExpression {
type: "MemberExpression"
};
interface Expression {
type: "Expression"
};

export interface AssignmentExpression {
type: "AssignmentExpression";
operator: AssignmentOperator;
down: Expression;
up: Expression;
left: Pattern | MemberExpression;
right: Expression;
nontraversable: RegExp;
}
20 changes: 20 additions & 0 deletions tests/lib/fixtures/new-keys-on-old-other-order.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
export type AssignmentOperator = "=";
interface Pattern {
type: "Pattern"
};
interface MemberExpression {
type: "MemberExpression"
};
interface Expression {
type: "Expression"
};

export interface AssignmentExpression {
type: "AssignmentExpression";
operator: AssignmentOperator;
up: Expression;
left: Pattern | MemberExpression;
down: Expression;
right: Expression;
nontraversable: RegExp;
}
35 changes: 35 additions & 0 deletions tests/lib/fixtures/new-keys-on-old.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
export type AssignmentOperator = "=";

interface IgnoreBase {
type: "Line";
}

type AnotherIgnore = IgnoreBase;

interface BasePattern {
type: "Pattern"
};
interface IgnoreChild extends Omit<BasePattern, "type"> {
};

interface Pattern {
type: "Pattern"
};
interface MemberExpression {
type: "MemberExpression"
};
interface Expression {
type: "Expression"
};

export interface AssignmentExpression {
type: "AssignmentExpression";
ignore: IgnoreChild;
anotherIgnore: AnotherIgnore;
operator: AssignmentOperator;
up: Expression;
down: Expression;
left: Pattern | MemberExpression;
right: Expression;
nontraversable: RegExp;
}
19 changes: 19 additions & 0 deletions tests/lib/fixtures/new-keys.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
export type AssignmentOperator = "=";
interface Pattern {
type: "Pattern"
};
interface MemberExpression {
type: "MemberExpression"
};
interface Expression {
type: "Expression"
};

export interface NewFangledExpression {
type: "NewFangledExpression";
operator: AssignmentOperator;
up: Expression;
down: Expression;
left: Pattern | MemberExpression;
right: Expression;
}
11 changes: 11 additions & 0 deletions tests/lib/fixtures/union-omit.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
export interface IgnoredStatement {
type: "IgnoredStatement"
}
export interface AnotherStatement {
type: "AnotherStatement";
anotherToIgnore: IgnoredStatement;
}

export interface StaticBlock extends Omit<AnotherStatement, 'type' | 'anotherToIgnore'> {
type: "StaticBlock";
}