Skip to content

Commit 1579b65

Browse files
Copilotalexr00
Copilot
and
alexr00
authored
Add local ESLint rule: public methods must return well-defined types (no inline types including generics) (#7382)
* Initial plan * Initial analysis and plan for public methods well-defined types ESLint rule Co-authored-by: alexr00 <[email protected]> * Implement public-methods-well-defined-types ESLint rule Co-authored-by: alexr00 <[email protected]> * Add documentation and targeted configuration for ESLint rule * Revert yarn.lock changes as requested Co-authored-by: alexr00 <[email protected]> * Update lock file * Integrate ESLint rule into main configuration Co-authored-by: alexr00 <[email protected]> * Delete readme * Update ESLint rule to catch Promise with inline types Co-authored-by: alexr00 <[email protected]> --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: alexr00 <[email protected]>
1 parent afa199e commit 1579b65

File tree

6 files changed

+217
-13
lines changed

6 files changed

+217
-13
lines changed

‎.eslintrc.js

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
/*---------------------------------------------------------------------------------------------
2+
* Copyright (c) Microsoft Corporation. All rights reserved.
3+
* Licensed under the MIT License. See License.txt in the project root for license information.
4+
*--------------------------------------------------------------------------------------------*/
5+
6+
'use strict';
7+
8+
const RULES_DIR = require('eslint-plugin-rulesdir');
9+
RULES_DIR.RULES_DIR = './build/eslint-rules';
10+
11+
module.exports = {
12+
extends: ['.eslintrc.base.json'],
13+
env: {
14+
browser: true,
15+
node: true
16+
},
17+
parserOptions: {
18+
project: 'tsconfig.eslint.json'
19+
},
20+
plugins: ['rulesdir'],
21+
overrides: [
22+
{
23+
files: ['webviews/**/*.ts', 'webviews/**/*.tsx'],
24+
rules: {
25+
'rulesdir/public-methods-well-defined-types': 'error'
26+
}
27+
}
28+
]
29+
};

‎.eslintrc.json

Lines changed: 0 additions & 10 deletions
This file was deleted.

‎build/eslint-rules/index.js

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
/*---------------------------------------------------------------------------------------------
2+
* Copyright (c) Microsoft Corporation. All rights reserved.
3+
* Licensed under the MIT License. See License.txt in the project root for license information.
4+
*--------------------------------------------------------------------------------------------*/
5+
6+
'use strict';
7+
8+
module.exports = {
9+
'public-methods-well-defined-types': require('./public-methods-well-defined-types'),
10+
};

‎build/eslint-rules/public-methods-well-defined-types.js

Lines changed: 170 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,170 @@
1+
/*---------------------------------------------------------------------------------------------
2+
* Copyright (c) Microsoft Corporation. All rights reserved.
3+
* Licensed under the MIT License. See License.txt in the project root for license information.
4+
*--------------------------------------------------------------------------------------------*/
5+
6+
'use strict';
7+
8+
/**
9+
* ESLint rule to enforce that public methods in exported classes return well-defined types.
10+
* This rule ensures that no inline type (object literal, anonymous type, etc.) is returned
11+
* from any public method.
12+
*/
13+
14+
module.exports = {
15+
meta: {
16+
type: 'problem',
17+
docs: {
18+
description: 'Enforce that public methods return well-defined types (no inline types)',
19+
category: 'TypeScript',
20+
recommended: false,
21+
},
22+
schema: [],
23+
messages: {
24+
inlineReturnType: 'Public method "{{methodName}}" should return a well-defined type, not an inline type. Consider defining an interface or type alias.',
25+
},
26+
},
27+
28+
create(context) {
29+
/**
30+
* Check if a node represents an inline type that should be flagged
31+
*/
32+
function isInlineType(typeNode) {
33+
if (!typeNode) return false;
34+
35+
switch (typeNode.type) {
36+
// Object type literals: { foo: string, bar: number }
37+
case 'TSTypeLiteral':
38+
return true;
39+
40+
// Union types with inline object types: string | { foo: bar }
41+
case 'TSUnionType':
42+
return typeNode.types.some(isInlineType);
43+
44+
// Intersection types with inline object types: Base & { foo: bar }
45+
case 'TSIntersectionType':
46+
return typeNode.types.some(isInlineType);
47+
48+
// Tuple types: [string, number]
49+
case 'TSTupleType':
50+
return true;
51+
52+
// Mapped types: { [K in keyof T]: U }
53+
case 'TSMappedType':
54+
return true;
55+
56+
// Conditional types: T extends U ? X : Y (inline)
57+
case 'TSConditionalType':
58+
return true;
59+
60+
// Type references with inline type arguments: Promise<{x: string}>, Array<{y: number}>
61+
case 'TSTypeReference':
62+
// Check if any type arguments contain inline types
63+
if (typeNode.typeParameters && typeNode.typeParameters.params) {
64+
return typeNode.typeParameters.params.some(isInlineType);
65+
}
66+
return false;
67+
68+
default:
69+
return false;
70+
}
71+
}
72+
73+
/**
74+
* Check if a method is public (not private or protected)
75+
*/
76+
function isPublicMethod(node) {
77+
// If no accessibility modifier is specified, it's public by default
78+
if (!node.accessibility) return true;
79+
return node.accessibility === 'public';
80+
}
81+
82+
/**
83+
* Check if a class is exported
84+
*/
85+
function isExportedClass(node) {
86+
// Check if the class declaration itself is exported
87+
if (node.parent && node.parent.type === 'ExportNamedDeclaration') {
88+
return true;
89+
}
90+
// Check if it's a default export
91+
if (node.parent && node.parent.type === 'ExportDefaultDeclaration') {
92+
return true;
93+
}
94+
return false;
95+
}
96+
97+
return {
98+
MethodDefinition(node) {
99+
// Only check methods in exported classes
100+
const classNode = node.parent.parent; // MethodDefinition -> ClassBody -> ClassDeclaration
101+
if (!classNode || classNode.type !== 'ClassDeclaration' || !isExportedClass(classNode)) {
102+
return;
103+
}
104+
105+
// Only check public methods
106+
if (!isPublicMethod(node)) {
107+
return;
108+
}
109+
110+
// Check if the method has a return type annotation
111+
const functionNode = node.value;
112+
if (!functionNode.returnType) {
113+
return; // No explicit return type, skip
114+
}
115+
116+
const returnTypeNode = functionNode.returnType.typeAnnotation;
117+
118+
// Check if the return type is an inline type
119+
if (isInlineType(returnTypeNode)) {
120+
const methodName = node.key.type === 'Identifier' ? node.key.name : '<computed>';
121+
context.report({
122+
node: functionNode.returnType,
123+
messageId: 'inlineReturnType',
124+
data: {
125+
methodName: methodName,
126+
},
127+
});
128+
}
129+
},
130+
131+
// Also check arrow function properties that are public methods
132+
PropertyDefinition(node) {
133+
// Only check properties in exported classes
134+
const classNode = node.parent.parent; // PropertyDefinition -> ClassBody -> ClassDeclaration
135+
if (!classNode || classNode.type !== 'ClassDeclaration' || !isExportedClass(classNode)) {
136+
return;
137+
}
138+
139+
// Only check public methods
140+
if (!isPublicMethod(node)) {
141+
return;
142+
}
143+
144+
// Check if the property is an arrow function
145+
if (node.value && node.value.type === 'ArrowFunctionExpression') {
146+
const arrowFunction = node.value;
147+
148+
// Check if the arrow function has a return type annotation
149+
if (!arrowFunction.returnType) {
150+
return; // No explicit return type, skip
151+
}
152+
153+
const returnTypeNode = arrowFunction.returnType.typeAnnotation;
154+
155+
// Check if the return type is an inline type
156+
if (isInlineType(returnTypeNode)) {
157+
const methodName = node.key.type === 'Identifier' ? node.key.name : '<computed>';
158+
context.report({
159+
node: arrowFunction.returnType,
160+
messageId: 'inlineReturnType',
161+
data: {
162+
methodName: methodName,
163+
},
164+
});
165+
}
166+
}
167+
}
168+
};
169+
},
170+
};

‎package.json

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2161,7 +2161,7 @@
21612161
"command": "pr.copyVscodeDevPrLink",
21622162
"when": "github:inReviewMode && remoteName != codespaces && embedderIdentifier != github.dev"
21632163
},
2164-
{
2164+
{
21652165
"command": "pr.copyPrLink",
21662166
"when": "false"
21672167
},
@@ -3353,7 +3353,6 @@
33533353
"command": "pr.openSessionLogFromDescription",
33543354
"when": "webviewId == PullRequestOverview && github:codingAgentMenu"
33553355
}
3356-
33573356
],
33583357
"chat/chatSessions": [
33593358
{
@@ -4032,7 +4031,7 @@
40324031
"compile:test": "tsc -p tsconfig.test.json",
40334032
"compile:node": "webpack --mode development --config-name extension:node --config-name webviews",
40344033
"compile:web": "webpack --mode development --config-name extension:webworker --config-name webviews",
4035-
"lint": "eslint --fix --cache --config .eslintrc.json --ignore-pattern src/env/browser/**/* \"{src,webviews}/**/*.{ts,tsx}\"",
4034+
"lint": "eslint --fix --cache --config .eslintrc.js --ignore-pattern src/env/browser/**/* \"{src,webviews}/**/*.{ts,tsx}\"",
40364035
"lint:browser": "eslint --fix --cache --cache-location .eslintcache.browser --config .eslintrc.browser.json --ignore-pattern src/env/node/**/* \"{src,webviews}/**/*.{ts,tsx}\"",
40374036
"package": "npx vsce package --yarn",
40384037
"test": "yarn run test:preprocess && node ./out/src/test/runTests.js",
@@ -4075,6 +4074,7 @@
40754074
"eslint": "7.22.0",
40764075
"eslint-cli": "1.1.1",
40774076
"eslint-plugin-import": "2.22.1",
4077+
"eslint-plugin-rulesdir": "^0.2.2",
40784078
"event-stream": "^4.0.1",
40794079
"fork-ts-checker-webpack-plugin": "6.1.1",
40804080
"glob": "7.1.6",

‎yarn.lock

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2796,6 +2796,11 @@ [email protected]:
27962796
resolve "^1.17.0"
27972797
tsconfig-paths "^3.9.0"
27982798

2799+
eslint-plugin-rulesdir@^0.2.2:
2800+
version "0.2.2"
2801+
resolved "https://registry.yarnpkg.com/eslint-plugin-rulesdir/-/eslint-plugin-rulesdir-0.2.2.tgz#84756ec39cd8503b1fe8af6a02a5da361e2bd076"
2802+
integrity sha512-qhBtmrWgehAIQeMDJ+Q+PnOz1DWUZMPeVrI0wE9NZtnpIMFUfh3aPKFYt2saeMSemZRrvUtjWfYwepsC8X+mjQ==
2803+
27992804
[email protected], eslint-scope@^5.1.1:
28002805
version "5.1.1"
28012806
resolved "https://registry.yarnpkg.com/eslint-scope/-/eslint-scope-5.1.1.tgz#e786e59a66cb92b3f6c1fb0d508aab174848f48c"

0 commit comments

Comments
 (0)