Merge pull request #2936 from github/koesie10/method-modeling-panel-v…

…alidation

Add model validation in method modeling panel

extensions/ql-vscode/src/model-editor/shared/validation.ts

@@ -0,0 +1,145 @@
+import { ModeledMethod } from "../modeled-method";
+import { MethodSignature } from "../method";
+import { assertNever } from "../../common/helpers-pure";
+
+export type ModeledMethodValidationError = {
+  title: string;
+  message: string;
+  actionText: string;
+  index: number;
+};
+
+/**
+ * This method will reset any properties which are not used for the specific type of modeled method.
+ *
+ * It will also set the `provenance` to `manual` since multiple modelings of the same method with a
+ * different provenance are not actually different.
+ *
+ * The returned canonical modeled method should only be used for comparisons. It should not be used
+ * for display purposes, saving the model, or any other purpose which requires the original modeled
+ * method to be preserved.
+ *
+ * @param modeledMethod The modeled method to canonicalize
+ */
+function canonicalizeModeledMethod(
+  modeledMethod: ModeledMethod,
+): ModeledMethod {
+  const methodSignature: MethodSignature = {
+    signature: modeledMethod.signature,
+    packageName: modeledMethod.packageName,
+    typeName: modeledMethod.typeName,
+    methodName: modeledMethod.methodName,
+    methodParameters: modeledMethod.methodParameters,
+  };
+
+  switch (modeledMethod.type) {
+    case "none":
+      return {
+        ...methodSignature,
+        type: "none",
+        input: "",
+        output: "",
+        kind: "",
+        provenance: "manual",
+      };
+    case "source":
+      return {
+        ...methodSignature,
+        type: "source",
+        input: "",
+        output: modeledMethod.output,
+        kind: modeledMethod.kind,
+        provenance: "manual",
+      };
+    case "sink":
+      return {
+        ...methodSignature,
+        type: "sink",
+        input: modeledMethod.input,
+        output: "",
+        kind: modeledMethod.kind,
+        provenance: "manual",
+      };
+    case "summary":
+      return {
+        ...methodSignature,
+        type: "summary",
+        input: modeledMethod.input,
+        output: modeledMethod.output,
+        kind: modeledMethod.kind,
+        provenance: "manual",
+      };
+    case "neutral":
+      return {
+        ...methodSignature,
+        type: "neutral",
+        input: "",
+        output: "",
+        kind: "",
+        provenance: "manual",
+      };
+    default:
+      assertNever(modeledMethod.type);
+  }
+}
+
+export function validateModeledMethods(
+  modeledMethods: ModeledMethod[],
+): ModeledMethodValidationError[] {
+  // Anything that is not modeled will not be saved, so we don't need to validate it
+  const consideredModeledMethods = modeledMethods.filter(
+    (modeledMethod) => modeledMethod.type !== "none",
+  );
+
+  const result: ModeledMethodValidationError[] = [];
+
+  // If the same model is present multiple times, only the first one makes sense, so we should give
+  // an error for any duplicates.
+  const seenModeledMethods = new Set<string>();
+  for (const modeledMethod of consideredModeledMethods) {
+    const canonicalModeledMethod = canonicalizeModeledMethod(modeledMethod);
+    const key = JSON.stringify(
+      canonicalModeledMethod,
+      // This ensures the keys are always in the same order
+      Object.keys(canonicalModeledMethod).sort(),
+    );
+
+    if (seenModeledMethods.has(key)) {
+      result.push({
+        title: "Duplicated classification",
+        message:
+          "This method has two identical or conflicting classifications.",
+        actionText: "Modify or remove the duplicated classification.",
+        index: modeledMethods.indexOf(modeledMethod),
+      });
+    } else {
+      seenModeledMethods.add(key);
+    }
+  }
+
+  const neutralModeledMethod = consideredModeledMethods.find(
+    (modeledMethod) => modeledMethod.type === "neutral",
+  );
+  const hasNonNeutralModeledMethod = consideredModeledMethods.some(
+    (modeledMethod) => modeledMethod.type !== "neutral",
+  );
+
+  // If there is a neutral model and any other model, that is an error
+  if (neutralModeledMethod && hasNonNeutralModeledMethod) {
+    // Another validation will validate that only one neutral method is present, so we only need
+    // to return an error for the first one
+
+    result.push({
+      title: "Conflicting classification",
+      message:
+        "This method has a neutral classification, which conflicts with other classifications.",
+      actionText: "Modify or remove the neutral classification.",
+      index: modeledMethods.indexOf(neutralModeledMethod),
+    });
+  }
+
+  // Sort by index so that the errors are always in the same order
+  result.sort((a, b) => a.index - b.index);
+
+  return result;
+}

extensions/ql-vscode/src/stories/method-modeling/MethodModeling.stories.tsx

@@ -69,3 +69,35 @@ MultipleModelingsModeledMultiple.args = {
   showMultipleModels: true,
   modelingStatus: "saved",
 };
+
+export const MultipleModelingsValidationFailedNeutral = Template.bind({});
+MultipleModelingsValidationFailedNeutral.args = {
+  method,
+  modeledMethods: [
+    createModeledMethod(method),
+    createModeledMethod({
+      ...method,
+      type: "neutral",
+    }),
+  ],
+  showMultipleModels: true,
+  modelingStatus: "unsaved",
+};
+
+export const MultipleModelingsValidationFailedDuplicate = Template.bind({});
+MultipleModelingsValidationFailedDuplicate.args = {
+  method,
+  modeledMethods: [
+    createModeledMethod(method),
+    createModeledMethod({
+      ...method,
+      type: "source",
+      input: "",
+      output: "ReturnValue",
+      kind: "remote",
+    }),
+    createModeledMethod(method),
+  ],
+  showMultipleModels: true,
+  modelingStatus: "unsaved",
+};

extensions/ql-vscode/src/view/common/Alert.tsx

@@ -85,11 +85,30 @@ type Props = {
 
   // Inverse the color scheme
   inverse?: boolean;
+
+  /**
+   * Role is used as the ARIA role. "alert" should only be set if the alert requires
+   * the user's immediate attention. "status" should be set if the alert is not
+   * important enough to require the user's immediate attention.
+   *
+   * Can be left out if the alert is not important enough to require the user's
+   * immediate attention. In this case, no ARIA role will be set and the alert
+   * will be read as normal text. The user will not be notified about any changes
+   * to the alert.
+   */
+  role?: "alert" | "status";
 };
 
-export const Alert = ({ type, title, message, actions, inverse }: Props) => {
+export const Alert = ({
+  type,
+  title,
+  message,
+  actions,
+  inverse,
+  role,
+}: Props) => {
   return (
-    <Container type={type} inverse={inverse}>
+    <Container type={type} inverse={inverse} role={role}>
       <Title>
         {getTypeText(type)}: {title}
       </Title>

extensions/ql-vscode/src/view/method-modeling/ModeledMethodAlert.tsx

@@ -0,0 +1,30 @@
+import { ModeledMethodValidationError } from "../../model-editor/shared/validation";
+import TextButton from "../common/TextButton";
+import { Alert } from "../common";
+import * as React from "react";
+import { useCallback } from "react";
+
+type Props = {
+  error: ModeledMethodValidationError;
+  setSelectedIndex: (index: number) => void;
+};
+
+export const ModeledMethodAlert = ({ error, setSelectedIndex }: Props) => {
+  const handleClick = useCallback(() => {
+    setSelectedIndex(error.index);
+  }, [error.index, setSelectedIndex]);
+
+  return (
+    <Alert
+      role="alert"
+      type="error"
+      title={error.title}
+      message={
+        <>
+          {error.message}{" "}
+          <TextButton onClick={handleClick}>{error.actionText}</TextButton>
+        </>
+      }
+    />
+  );
+};

extensions/ql-vscode/src/view/method-modeling/MultipleModeledMethodsPanel.tsx

@@ -6,6 +6,8 @@ import { styled } from "styled-components";
 import { MethodModelingInputs } from "./MethodModelingInputs";
 import { VSCodeButton } from "@vscode/webview-ui-toolkit/react";
 import { Codicon } from "../common";
+import { validateModeledMethods } from "../../model-editor/shared/validation";
+import { ModeledMethodAlert } from "./ModeledMethodAlert";
 
 export type MultipleModeledMethodsPanelProps = {
   method: Method;
@@ -19,9 +21,14 @@ const Container = styled.div`
   gap: 0.25rem;
 
   padding-bottom: 0.5rem;
+  border-top: 0.05rem solid var(--vscode-panelSection-border);
   border-bottom: 0.05rem solid var(--vscode-panelSection-border);
 `;
 
+const AlertContainer = styled.div`
+  margin-top: 0.5rem;
+`;
+
 const Footer = styled.div`
   display: flex;
   flex-direction: row;
@@ -54,6 +61,11 @@ export const MultipleModeledMethodsPanel = ({
     setSelectedIndex((previousIndex) => previousIndex + 1);
   }, []);
 
+  const validationErrors = useMemo(
+    () => validateModeledMethods(modeledMethods),
+    [modeledMethods],
+  );
+
   const handleAddClick = useCallback(() => {
     const newModeledMethod: ModeledMethod = {
       type: "none",
@@ -110,6 +122,17 @@ export const MultipleModeledMethodsPanel = ({
 
   return (
     <Container>
+      {validationErrors.length > 0 && (
+        <AlertContainer>
+          {validationErrors.map((error, index) => (
+            <ModeledMethodAlert
+              key={index}
+              error={error}
+              setSelectedIndex={setSelectedIndex}
+            />
+          ))}
+        </AlertContainer>
+      )}
       {modeledMethods.length > 0 ? (
         <MethodModelingInputs
           method={method}