Skip to content

Commit b9b16a8

Browse files
committed
Add a consistency check for the model editor
1 parent eb1bf8f commit b9b16a8

6 files changed

Lines changed: 189 additions & 0 deletions

File tree

extensions/ql-vscode/src/config.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -712,12 +712,17 @@ const LLM_GENERATION_DEV_ENDPOINT = new Setting(
712712
);
713713
const EXTENSIONS_DIRECTORY = new Setting("extensionsDirectory", MODEL_SETTING);
714714
const ENABLE_RUBY = new Setting("enableRuby", MODEL_SETTING);
715+
const ENABLE_CONSISTENCY_CHECK = new Setting(
716+
"enableConsistencyCheck",
717+
MODEL_SETTING,
718+
);
715719

716720
export interface ModelConfig {
717721
flowGeneration: boolean;
718722
llmGeneration: boolean;
719723
getExtensionsDirectory(languageId: string): string | undefined;
720724
enableRuby: boolean;
725+
enableConsistencyCheck: boolean;
721726
}
722727

723728
export class ModelConfigListener extends ConfigListener implements ModelConfig {
@@ -758,6 +763,10 @@ export class ModelConfigListener extends ConfigListener implements ModelConfig {
758763
public get enableRuby(): boolean {
759764
return !!ENABLE_RUBY.getValue<boolean>();
760765
}
766+
767+
public get enableConsistencyCheck(): boolean {
768+
return !!ENABLE_CONSISTENCY_CHECK.getValue<boolean>();
769+
}
761770
}
762771

763772
const GITHUB_DATABASE_SETTING = new Setting("githubDatabase", ROOT_SETTING);
Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
import { Method } from "./method";
2+
import { ModeledMethod } from "./modeled-method";
3+
import { BaseLogger } from "../common/logging";
4+
5+
interface Notifier {
6+
missingMethod(signature: string): void;
7+
inconsistentSupported(
8+
signature: string,
9+
expectedSupported: boolean,
10+
actualSupported: boolean,
11+
): void;
12+
}
13+
14+
export function checkConsistency(
15+
methods: readonly Method[],
16+
modeledMethods: Readonly<Record<string, readonly ModeledMethod[]>>,
17+
notifier: Notifier,
18+
) {
19+
const methodsBySignature = methods.reduce(
20+
(acc, method) => {
21+
acc[method.signature] = method;
22+
return acc;
23+
},
24+
{} as Record<string, Method>,
25+
);
26+
27+
for (const signature in modeledMethods) {
28+
const method = methodsBySignature[signature];
29+
if (!method) {
30+
notifier.missingMethod(signature);
31+
continue;
32+
}
33+
34+
const modeledMethodsForSignature = modeledMethods[signature];
35+
36+
checkMethodConsistency(method, modeledMethodsForSignature, notifier);
37+
}
38+
}
39+
40+
function checkMethodConsistency(
41+
method: Method,
42+
modeledMethods: readonly ModeledMethod[],
43+
notifier: Notifier,
44+
) {
45+
const expectSupported = modeledMethods.some((m) => m.type !== "none");
46+
47+
if (method.supported !== expectSupported) {
48+
notifier.inconsistentSupported(
49+
method.signature,
50+
expectSupported,
51+
method.supported,
52+
);
53+
}
54+
}
55+
56+
export class DefaultNotifier implements Notifier {
57+
constructor(private readonly logger: BaseLogger) {}
58+
59+
missingMethod(signature: string) {
60+
void this.logger.log(
61+
`Consistency check: Missing method ${signature} for method that is modeled`,
62+
);
63+
}
64+
65+
inconsistentSupported(
66+
signature: string,
67+
expectedSupported: boolean,
68+
actualSupported: boolean,
69+
) {
70+
void this.logger.log(
71+
`Consistency check: Inconsistent supported flag for method ${signature}. Expected supported: ${expectedSupported}, actual supported: ${actualSupported}`,
72+
);
73+
}
74+
}

extensions/ql-vscode/src/model-editor/model-editor-module.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import { ModelingEvents } from "./modeling-events";
2424
import { getModelsAsDataLanguage } from "./languages";
2525
import { INITIAL_MODE } from "./shared/mode";
2626
import { isSupportedLanguage } from "./supported-languages";
27+
import { DefaultNotifier, checkConsistency } from "./consistency-check";
2728

2829
export class ModelEditorModule extends DisposableObject {
2930
private readonly queryStorageDir: string;
@@ -99,6 +100,24 @@ export class ModelEditorModule extends DisposableObject {
99100
await this.showMethod(event.databaseItem, event.method, event.usage);
100101
}),
101102
);
103+
104+
this.push(
105+
this.modelingEvents.onMethodsChanged((event) => {
106+
if (!this.modelConfig.enableConsistencyCheck) {
107+
return;
108+
}
109+
110+
const modeledMethods = this.modelingStore.getModeledMethods(
111+
event.databaseItem,
112+
);
113+
114+
checkConsistency(
115+
event.methods,
116+
modeledMethods,
117+
new DefaultNotifier(this.app.logger),
118+
);
119+
}),
120+
);
102121
}
103122

104123
private async showMethod(

extensions/ql-vscode/src/model-editor/modeling-events.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import { Mode } from "./shared/mode";
99
interface MethodsChangedEvent {
1010
readonly methods: readonly Method[];
1111
readonly dbUri: string;
12+
readonly databaseItem: DatabaseItem;
1213
readonly isActiveDb: boolean;
1314
}
1415

@@ -166,10 +167,12 @@ export class ModelingEvents extends DisposableObject {
166167
public fireMethodsChangedEvent(
167168
methods: Method[],
168169
dbUri: string,
170+
databaseItem: DatabaseItem,
169171
isActiveDb: boolean,
170172
) {
171173
this.onMethodsChangedEventEmitter.fire({
172174
methods,
175+
databaseItem,
173176
dbUri,
174177
isActiveDb,
175178
});

extensions/ql-vscode/src/model-editor/modeling-store.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,7 @@ export class ModelingStore extends DisposableObject {
155155
this.modelingEvents.fireMethodsChangedEvent(
156156
methods,
157157
dbUri,
158+
dbItem,
158159
dbUri === this.activeDb,
159160
);
160161
}
Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
import { checkConsistency } from "../../../src/model-editor/consistency-check";
2+
import { createSourceModeledMethod } from "../../factories/model-editor/modeled-method-factories";
3+
import { createMethod } from "../../factories/model-editor/method-factories";
4+
5+
describe("checkConsistency", () => {
6+
const notifier = {
7+
missingMethod: jest.fn(),
8+
inconsistentSupported: jest.fn(),
9+
};
10+
11+
beforeEach(() => {
12+
notifier.missingMethod.mockReset();
13+
notifier.inconsistentSupported.mockReset();
14+
});
15+
16+
it("should call missingMethod when method is missing", () => {
17+
checkConsistency(
18+
[],
19+
{
20+
"Microsoft.CodeAnalysis.CSharp.SyntaxFactory.SeparatedList`1(System.Collections.Generic.IEnumerable<TNode>)":
21+
[createSourceModeledMethod()],
22+
},
23+
notifier,
24+
);
25+
26+
expect(notifier.missingMethod).toHaveBeenCalledWith(
27+
"Microsoft.CodeAnalysis.CSharp.SyntaxFactory.SeparatedList`1(System.Collections.Generic.IEnumerable<TNode>)",
28+
);
29+
expect(notifier.inconsistentSupported).not.toHaveBeenCalled();
30+
});
31+
32+
it("should call inconsistentSupported when support is inconsistent", () => {
33+
checkConsistency(
34+
[
35+
createMethod({
36+
signature:
37+
"Microsoft.CodeAnalysis.CSharp.SyntaxFactory.SeparatedList`1(System.Collections.Generic.IEnumerable<TNode>)",
38+
packageName: "Microsoft.CodeAnalysis.CSharp",
39+
typeName: "SyntaxFactory",
40+
methodName: "SeparatedList`1",
41+
methodParameters: "(System.Collections.Generic.IEnumerable<TNode>)",
42+
supported: false,
43+
}),
44+
],
45+
{
46+
"Microsoft.CodeAnalysis.CSharp.SyntaxFactory.SeparatedList`1(System.Collections.Generic.IEnumerable<TNode>)":
47+
[createSourceModeledMethod({})],
48+
},
49+
notifier,
50+
);
51+
52+
expect(notifier.inconsistentSupported).toHaveBeenCalledWith(
53+
"Microsoft.CodeAnalysis.CSharp.SyntaxFactory.SeparatedList`1(System.Collections.Generic.IEnumerable<TNode>)",
54+
true,
55+
false,
56+
);
57+
expect(notifier.missingMethod).not.toHaveBeenCalled();
58+
});
59+
60+
it("should call no methods when consistent", () => {
61+
checkConsistency(
62+
[
63+
createMethod({
64+
signature:
65+
"Microsoft.CodeAnalysis.CSharp.SyntaxFactory.SeparatedList<TNode>(System.Collections.Generic.IEnumerable<TNode>)",
66+
packageName: "Microsoft.CodeAnalysis.CSharp",
67+
typeName: "SyntaxFactory",
68+
methodName: "SeparatedList<TNode>",
69+
methodParameters: "(System.Collections.Generic.IEnumerable<TNode>)",
70+
supported: true,
71+
}),
72+
],
73+
{
74+
"Microsoft.CodeAnalysis.CSharp.SyntaxFactory.SeparatedList<TNode>(System.Collections.Generic.IEnumerable<TNode>)":
75+
[createSourceModeledMethod({})],
76+
},
77+
notifier,
78+
);
79+
80+
expect(notifier.missingMethod).not.toHaveBeenCalled();
81+
expect(notifier.inconsistentSupported).not.toHaveBeenCalled();
82+
});
83+
});

0 commit comments

Comments
 (0)