Merge pull request #3141 from github/koesie10/model-editor-consistency-check

Add a consistency check for the model editor

Author: koesie10

extensions/ql-vscode/src/model-editor/consistency-check.ts

@@ -0,0 +1,69 @@
+import { Method } from "./method";
+import { ModeledMethod } from "./modeled-method";
+import { BaseLogger } from "../common/logging";
+
+interface Notifier {
+  missingMethod(signature: string): void;
+  inconsistentSupported(signature: string, expectedSupported: boolean): void;
+}
+
+export function checkConsistency(
+  methods: readonly Method[],
+  modeledMethods: Readonly<Record<string, readonly ModeledMethod[]>>,
+  notifier: Notifier,
+) {
+  const methodsBySignature = methods.reduce(
+    (acc, method) => {
+      acc[method.signature] = method;
+      return acc;
+    },
+    {} as Record<string, Method>,
+  );
+
+  for (const signature in modeledMethods) {
+    const method = methodsBySignature[signature];
+    if (!method) {
+      notifier.missingMethod(signature);
+      continue;
+    }
+
+    const modeledMethodsForSignature = modeledMethods[signature];
+
+    checkMethodConsistency(method, modeledMethodsForSignature, notifier);
+  }
+}
+
+function checkMethodConsistency(
+  method: Method,
+  modeledMethods: readonly ModeledMethod[],
+  notifier: Notifier,
+) {
+  // Type models are currently not shown as `supported` since they do not give any model information.
+  const expectSupported = modeledMethods.some(
+    (m) => m.type !== "none" && m.type !== "type",
+  );
+
+  if (method.supported !== expectSupported) {
+    notifier.inconsistentSupported(method.signature, expectSupported);
+  }
+}
+
+export class DefaultNotifier implements Notifier {
+  constructor(private readonly logger: BaseLogger) {}
+
+  missingMethod(signature: string) {
+    void this.logger.log(
+      `Model editor query consistency check: Missing method ${signature} for method that is modeled.`,
+    );
+  }
+
+  inconsistentSupported(signature: string, expectedSupported: boolean) {
+    const expectedMessage = expectedSupported
+      ? `Expected method to be supported, but it is not.`
+      : `Expected method to not be supported, but it is.`;
+
+    void this.logger.log(
+      `Model editor query consistency check: Inconsistent supported flag for method ${signature}. ${expectedMessage}`,
+    );
+  }
+}

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

@@ -24,6 +24,7 @@ import { ModelingEvents } from "./modeling-events";
 import { getModelsAsDataLanguage } from "./languages";
 import { INITIAL_MODE } from "./shared/mode";
 import { isSupportedLanguage } from "./supported-languages";
+import { DefaultNotifier, checkConsistency } from "./consistency-check";
 
 export class ModelEditorModule extends DisposableObject {
   private readonly queryStorageDir: string;
@@ -99,6 +100,20 @@ export class ModelEditorModule extends DisposableObject {
         await this.showMethod(event.databaseItem, event.method, event.usage);
       }),
     );
+
+    this.push(
+      this.modelingEvents.onMethodsChanged((event) => {
+        const modeledMethods = this.modelingStore.getModeledMethods(
+          event.databaseItem,
+        );
+
+        checkConsistency(
+          event.methods,
+          modeledMethods,
+          new DefaultNotifier(this.app.logger),
+        );
+      }),
+    );
   }
 
   private async showMethod(

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

@@ -9,6 +9,7 @@ import { Mode } from "./shared/mode";
 interface MethodsChangedEvent {
   readonly methods: readonly Method[];
   readonly dbUri: string;
+  readonly databaseItem: DatabaseItem;
   readonly isActiveDb: boolean;
 }
 
@@ -166,10 +167,12 @@ export class ModelingEvents extends DisposableObject {
   public fireMethodsChangedEvent(
     methods: Method[],
     dbUri: string,
+    databaseItem: DatabaseItem,
     isActiveDb: boolean,
   ) {
     this.onMethodsChangedEventEmitter.fire({
       methods,
+      databaseItem,
       dbUri,
       isActiveDb,
     });

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

@@ -155,6 +155,7 @@ export class ModelingStore extends DisposableObject {
     this.modelingEvents.fireMethodsChangedEvent(
       methods,
       dbUri,
+      dbItem,
       dbUri === this.activeDb,
     );
   }

extensions/ql-vscode/test/unit-tests/model-editor/consistency-check.test.ts

@@ -0,0 +1,82 @@
+import { checkConsistency } from "../../../src/model-editor/consistency-check";
+import { createSourceModeledMethod } from "../../factories/model-editor/modeled-method-factories";
+import { createMethod } from "../../factories/model-editor/method-factories";
+
+describe("checkConsistency", () => {
+  const notifier = {
+    missingMethod: jest.fn(),
+    inconsistentSupported: jest.fn(),
+  };
+
+  beforeEach(() => {
+    notifier.missingMethod.mockReset();
+    notifier.inconsistentSupported.mockReset();
+  });
+
+  it("should call missingMethod when method is missing", () => {
+    checkConsistency(
+      [],
+      {
+        "Microsoft.CodeAnalysis.CSharp.SyntaxFactory.SeparatedList`1(System.Collections.Generic.IEnumerable<TNode>)":
+          [createSourceModeledMethod()],
+      },
+      notifier,
+    );
+
+    expect(notifier.missingMethod).toHaveBeenCalledWith(
+      "Microsoft.CodeAnalysis.CSharp.SyntaxFactory.SeparatedList`1(System.Collections.Generic.IEnumerable<TNode>)",
+    );
+    expect(notifier.inconsistentSupported).not.toHaveBeenCalled();
+  });
+
+  it("should call inconsistentSupported when support is inconsistent", () => {
+    checkConsistency(
+      [
+        createMethod({
+          signature:
+            "Microsoft.CodeAnalysis.CSharp.SyntaxFactory.SeparatedList`1(System.Collections.Generic.IEnumerable<TNode>)",
+          packageName: "Microsoft.CodeAnalysis.CSharp",
+          typeName: "SyntaxFactory",
+          methodName: "SeparatedList`1",
+          methodParameters: "(System.Collections.Generic.IEnumerable<TNode>)",
+          supported: false,
+        }),
+      ],
+      {
+        "Microsoft.CodeAnalysis.CSharp.SyntaxFactory.SeparatedList`1(System.Collections.Generic.IEnumerable<TNode>)":
+          [createSourceModeledMethod({})],
+      },
+      notifier,
+    );
+
+    expect(notifier.inconsistentSupported).toHaveBeenCalledWith(
+      "Microsoft.CodeAnalysis.CSharp.SyntaxFactory.SeparatedList`1(System.Collections.Generic.IEnumerable<TNode>)",
+      true,
+    );
+    expect(notifier.missingMethod).not.toHaveBeenCalled();
+  });
+
+  it("should call no methods when consistent", () => {
+    checkConsistency(
+      [
+        createMethod({
+          signature:
+            "Microsoft.CodeAnalysis.CSharp.SyntaxFactory.SeparatedList<TNode>(System.Collections.Generic.IEnumerable<TNode>)",
+          packageName: "Microsoft.CodeAnalysis.CSharp",
+          typeName: "SyntaxFactory",
+          methodName: "SeparatedList<TNode>",
+          methodParameters: "(System.Collections.Generic.IEnumerable<TNode>)",
+          supported: true,
+        }),
+      ],
+      {
+        "Microsoft.CodeAnalysis.CSharp.SyntaxFactory.SeparatedList<TNode>(System.Collections.Generic.IEnumerable<TNode>)":
+          [createSourceModeledMethod({})],
+      },
+      notifier,
+    );
+
+    expect(notifier.missingMethod).not.toHaveBeenCalled();
+    expect(notifier.inconsistentSupported).not.toHaveBeenCalled();
+  });
+});