Skip to content

Commit 0c6a4f9

Browse files
authored
feat(lint): Add custom ESLint rules for protobuf type checking for protobus ServiceClients (RooCodeInc#3946)
* Fix linter warnings in the webview (part 2) Replace protobus calls using object literals to use Message.create({...}) Fix incorrect property name detected after this change in webview-ui/src/components/settings/SettingsView.tsx Optimised imports in vscode. * formatting * feat(lint): Add custom ESLint rules for protobuf type checking Add two custom ESLint rules to enforce proper usage patterns when creating protobuf objects. Using .create() to build protobufs ensures that the protobuf is type checked when it is created. Protobufs created using object literals are not type checked, which can lead to subtle bugs and type mismatches. The linter rules detect when protobufs are created without using .create() or .fromPartial(). - no-protobuf-object-literals: Enforces the use of `.create()` or `.fromPartial()` methods instead of object literals when creating protobuf types. ``` /Users/sjf/cline/src/shared/proto-conversions/state/chat-settings-conversion.ts 9:9 warning Use ChatSettings.create() or ChatSettings.fromPartial() instead of object literal for protobuf type Found: return { mode: chatSettings.mode === "plan" ? PlanActMode.PLAN : PlanActMode.ACT, preferredLanguage: chatSettings.preferredLanguage, openAiReasoningEffort: chatSettings.openAIReasoningEffort, } Suggestion: ChatSettings.create({ mode: chatSettings.mode === "plan" ? PlanActMode.PLAN : PlanActMode.ACT, preferredLanguage: chatSettings.preferredLanguage, openAiReasoningEffort: chatSettings.openAIReasoningEffort, }) ``` - no-grpc-client-object-literals: Enforces proper protobuf creation for gRPC service client parameters. This needs a separate rule because the type signatures of the ServiceClients methods are too generic to be detected by the previous rule. ``` /Users/sjf/cline/webview-ui/src/components/mcp/configuration/tabs/add-server/AddRemoteServerForm.tsx 41:62 warning Use the appropriate protobuf .create() or .fromPartial() method instead of object literal for gRPC client parameters. Found: McpServiceClient.addRemoteMcpServer({ serverName: serverName.trim(), serverUrl: serverUrl.trim(), }) ``` These rules help maintain code quality by enforcing consistent patterns for working with protocol buffers throughout the codebase, reducing potential runtime errors from improper message construction. * Update test * Add custom eslint rules to new webview-ui config * Only include webview grpc ServiceClient check * Fix lint errors * formatting * Update package-lock.json * Update package.json
1 parent c1e38e6 commit 0c6a4f9

File tree

13 files changed

+3587
-51
lines changed

13 files changed

+3587
-51
lines changed

.eslintrc.json

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
"ecmaVersion": 6,
66
"sourceType": "module"
77
},
8-
"plugins": ["@typescript-eslint"],
8+
"plugins": ["@typescript-eslint", "eslint-rules"],
99
"rules": {
1010
"@typescript-eslint/naming-convention": [
1111
"warn",
@@ -19,7 +19,8 @@
1919
"eqeqeq": "warn",
2020
"no-throw-literal": "warn",
2121
"semi": "off",
22-
"react-hooks/exhaustive-deps": "off"
22+
"react-hooks/exhaustive-deps": "off",
23+
"eslint-rules/no-grpc-client-object-literals": "error"
2324
},
2425
"ignorePatterns": ["out", "dist", "**/*.d.ts"]
2526
}
Lines changed: 174 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,174 @@
1+
const { RuleTester: GrpcRuleTester } = require("eslint")
2+
const grpcRule = require("../no-grpc-client-object-literals")
3+
4+
const grpcRuleTester = new GrpcRuleTester({
5+
parser: require.resolve("@typescript-eslint/parser"),
6+
parserOptions: {
7+
ecmaVersion: 2020,
8+
sourceType: "module",
9+
ecmaFeatures: {
10+
jsx: true,
11+
},
12+
},
13+
})
14+
15+
grpcRuleTester.run("no-grpc-client-object-literals", grpcRule, {
16+
valid: [
17+
// Valid case: Using .create() method with gRPC client
18+
{
19+
code: `
20+
import { TogglePlanActModeRequest } from '@shared/proto/state';
21+
import { StateServiceClient } from '../services/grpc-client';
22+
23+
StateServiceClient.togglePlanActMode(
24+
TogglePlanActModeRequest.create({
25+
chatSettings: {
26+
mode: PlanActMode.PLAN,
27+
preferredLanguage: 'en',
28+
},
29+
})
30+
);
31+
`,
32+
},
33+
// Valid case: Using .fromPartial() method with gRPC client
34+
{
35+
code: `
36+
import { TogglePlanActModeRequest, ChatSettings } from '@shared/proto/state';
37+
import { StateServiceClient } from '../services/grpc-client';
38+
39+
const chatSettings = ChatSettings.fromPartial({
40+
mode: PlanActMode.PLAN,
41+
preferredLanguage: 'en',
42+
});
43+
44+
StateServiceClient.togglePlanActMode(
45+
TogglePlanActModeRequest.create({
46+
chatSettings: chatSettings,
47+
})
48+
);
49+
`,
50+
},
51+
// Valid case: Regular function call with object literal (not a gRPC client)
52+
{
53+
code: `
54+
function processData(data) {
55+
console.log(data);
56+
}
57+
58+
processData({
59+
id: 123,
60+
name: 'test',
61+
});
62+
`,
63+
},
64+
// Valid case: Using proper nested protobuf objects
65+
{
66+
code: `
67+
import { TogglePlanActModeRequest, ChatSettings } from '@shared/proto/state';
68+
import { StateServiceClient } from '../services/grpc-client';
69+
70+
// Using proper nested protobuf objects
71+
const chatSettings = ChatSettings.create({
72+
mode: 0,
73+
preferredLanguage: 'en',
74+
});
75+
76+
const request = TogglePlanActModeRequest.create({
77+
chatSettings: chatSettings,
78+
});
79+
80+
StateServiceClient.togglePlanActMode(request);
81+
`,
82+
},
83+
// Valid case: Object literal in second parameter (should not be checked)
84+
{
85+
code: `
86+
import { StateSubscribeRequest } from '@shared/proto/state';
87+
import { StateServiceClient } from '../services/grpc-client';
88+
89+
const request = StateSubscribeRequest.create({
90+
topics: ['apiConfig', 'tasks']
91+
});
92+
93+
// Second parameter is an object literal but should not trigger the rule
94+
StateServiceClient.subscribe(request, {
95+
metadata: {
96+
userId: 123,
97+
sessionId: "abc-123"
98+
}
99+
});
100+
`,
101+
},
102+
],
103+
invalid: [
104+
// Invalid case: Using object literal directly with gRPC client
105+
{
106+
code: `
107+
import { StateServiceClient } from '../services/grpc-client';
108+
109+
StateServiceClient.togglePlanActMode({
110+
chatSettings: {
111+
mode: 0,
112+
preferredLanguage: 'en',
113+
},
114+
});
115+
`,
116+
errors: [{ messageId: "useProtobufMethod" }],
117+
},
118+
// Invalid case: Using object literal with nested properties
119+
{
120+
code: `
121+
import { ChatSettings } from '@shared/proto/state';
122+
import { StateServiceClient } from '../services/grpc-client';
123+
124+
const chatSettings = ChatSettings.create({
125+
mode: 0,
126+
preferredLanguage: 'en',
127+
});
128+
129+
StateServiceClient.togglePlanActMode({
130+
chatSettings: {
131+
mode: 1,
132+
preferredLanguage: 'fr',
133+
},
134+
});
135+
`,
136+
errors: [{ messageId: "useProtobufMethod" }],
137+
},
138+
// Invalid case: Nested object literal in protobuf create method
139+
{
140+
code: `
141+
import { TogglePlanActModeRequest, ChatSettings } from '@shared/proto/state';
142+
import { StateServiceClient } from '../services/grpc-client';
143+
144+
// Using nested object literal instead of ChatSettings.create()
145+
const request = TogglePlanActModeRequest.create({
146+
chatSettings: {
147+
mode: 0,
148+
preferredLanguage: 'en',
149+
},
150+
});
151+
152+
StateServiceClient.togglePlanActMode(request);
153+
`,
154+
errors: [{ messageId: "useProtobufMethod" }],
155+
},
156+
// Invalid case: Object literal as first parameter to subscribe method
157+
{
158+
code: `
159+
import { StateServiceClient } from '../services/grpc-client';
160+
161+
// First parameter is an object literal, which should trigger the rule
162+
StateServiceClient.subscribe({
163+
topics: ['apiConfig', 'tasks']
164+
}, {
165+
metadata: {
166+
userId: 123,
167+
sessionId: "abc-123"
168+
}
169+
});
170+
`,
171+
errors: [{ messageId: "useProtobufMethod" }],
172+
},
173+
],
174+
})

eslint-rules/index.js

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
// eslint-rules/index.js
2+
const noGrpcClientObjectLiterals = require("./no-grpc-client-object-literals")
3+
4+
module.exports = {
5+
rules: {
6+
"no-grpc-client-object-literals": noGrpcClientObjectLiterals,
7+
},
8+
configs: {
9+
recommended: {
10+
plugins: ["local"],
11+
rules: {
12+
"local/no-grpc-client-object-literals": "error",
13+
},
14+
},
15+
},
16+
}

0 commit comments

Comments
 (0)