From 1606829cebad4543fd54aacc11dd2ce8e34c9e7d Mon Sep 17 00:00:00 2001
From: Robert <robertbrignull@github.com>
Date: Wed, 18 Oct 2023 16:04:40 +0100
Subject: [PATCH 1/5] Make setSelectedIndex optional

---
 .../src/view/method-modeling/ModeledMethodAlert.tsx    | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/extensions/ql-vscode/src/view/method-modeling/ModeledMethodAlert.tsx b/extensions/ql-vscode/src/view/method-modeling/ModeledMethodAlert.tsx
index 111109d3534..7a638a22c8d 100644
--- a/extensions/ql-vscode/src/view/method-modeling/ModeledMethodAlert.tsx
+++ b/extensions/ql-vscode/src/view/method-modeling/ModeledMethodAlert.tsx
@@ -6,12 +6,12 @@ import { useCallback } from "react";
 
 type Props = {
   error: ModeledMethodValidationError;
-  setSelectedIndex: (index: number) => void;
+  setSelectedIndex?: (index: number) => void;
 };
 
 export const ModeledMethodAlert = ({ error, setSelectedIndex }: Props) => {
   const handleClick = useCallback(() => {
-    setSelectedIndex(error.index);
+    setSelectedIndex?.(error.index);
   }, [error.index, setSelectedIndex]);
 
   return (
@@ -22,7 +22,11 @@ export const ModeledMethodAlert = ({ error, setSelectedIndex }: Props) => {
       message={
         <>
           {error.message}{" "}
-          <TextButton onClick={handleClick}>{error.actionText}</TextButton>
+          {setSelectedIndex ? (
+            <TextButton onClick={handleClick}>{error.actionText}</TextButton>
+          ) : (
+            error.actionText
+          )}
         </>
       }
     />

From c69a310110b6dd9792b89a61d29fd0978c21e928 Mon Sep 17 00:00:00 2001
From: Robert <robertbrignull@github.com>
Date: Thu, 19 Oct 2023 12:52:18 +0100
Subject: [PATCH 2/5] Add <> around inputs

---
 .../src/view/model-editor/MethodRow.tsx       | 109 +++++++++---------
 1 file changed, 56 insertions(+), 53 deletions(-)

diff --git a/extensions/ql-vscode/src/view/model-editor/MethodRow.tsx b/extensions/ql-vscode/src/view/model-editor/MethodRow.tsx
index 1125d86c9ec..a304920cc5d 100644
--- a/extensions/ql-vscode/src/view/model-editor/MethodRow.tsx
+++ b/extensions/ql-vscode/src/view/model-editor/MethodRow.tsx
@@ -200,61 +200,64 @@ const ModelableMethodRow = forwardRef<HTMLElement | undefined, MethodRowProps>(
             )}
           </>
         )}
-        {!props.modelingInProgress &&
-          modeledMethods.map((modeledMethod, index) => (
-            <Fragment key={index}>
-              <DataGridCell>
-                <ModelTypeDropdown
-                  method={method}
-                  modeledMethod={modeledMethod}
-                  onChange={modeledMethodChangedHandlers[index]}
-                />
-              </DataGridCell>
-              <DataGridCell>
-                <ModelInputDropdown
-                  method={method}
-                  modeledMethod={modeledMethod}
-                  onChange={modeledMethodChangedHandlers[index]}
-                />
-              </DataGridCell>
-              <DataGridCell>
-                <ModelOutputDropdown
-                  method={method}
-                  modeledMethod={modeledMethod}
-                  onChange={modeledMethodChangedHandlers[index]}
-                />
-              </DataGridCell>
-              <DataGridCell>
-                <ModelKindDropdown
-                  method={method}
-                  modeledMethod={modeledMethod}
-                  onChange={modeledMethodChangedHandlers[index]}
-                />
-              </DataGridCell>
-              {viewState.showMultipleModels && (
+        {!props.modelingInProgress && (
+          <>
+            {modeledMethods.map((modeledMethod, index) => (
+              <Fragment key={index}>
                 <DataGridCell>
-                  {index === modeledMethods.length - 1 ? (
-                    <CodiconRow
-                      appearance="icon"
-                      aria-label="Add new model"
-                      onClick={handleAddModelClick}
-                      disabled={addModelButtonDisabled}
-                    >
-                      <Codicon name="add" />
-                    </CodiconRow>
-                  ) : (
-                    <CodiconRow
-                      appearance="icon"
-                      aria-label="Remove model"
-                      onClick={removeModelClickedHandlers[index]}
-                    >
-                      <Codicon name="trash" />
-                    </CodiconRow>
-                  )}
+                  <ModelTypeDropdown
+                    method={method}
+                    modeledMethod={modeledMethod}
+                    onChange={modeledMethodChangedHandlers[index]}
+                  />
                 </DataGridCell>
-              )}
-            </Fragment>
-          ))}
+                <DataGridCell>
+                  <ModelInputDropdown
+                    method={method}
+                    modeledMethod={modeledMethod}
+                    onChange={modeledMethodChangedHandlers[index]}
+                  />
+                </DataGridCell>
+                <DataGridCell>
+                  <ModelOutputDropdown
+                    method={method}
+                    modeledMethod={modeledMethod}
+                    onChange={modeledMethodChangedHandlers[index]}
+                  />
+                </DataGridCell>
+                <DataGridCell>
+                  <ModelKindDropdown
+                    method={method}
+                    modeledMethod={modeledMethod}
+                    onChange={modeledMethodChangedHandlers[index]}
+                  />
+                </DataGridCell>
+                {viewState.showMultipleModels && (
+                  <DataGridCell>
+                    {index === modeledMethods.length - 1 ? (
+                      <CodiconRow
+                        appearance="icon"
+                        aria-label="Add new model"
+                        onClick={handleAddModelClick}
+                        disabled={addModelButtonDisabled}
+                      >
+                        <Codicon name="add" />
+                      </CodiconRow>
+                    ) : (
+                      <CodiconRow
+                        appearance="icon"
+                        aria-label="Remove model"
+                        onClick={removeModelClickedHandlers[index]}
+                      >
+                        <Codicon name="trash" />
+                      </CodiconRow>
+                    )}
+                  </DataGridCell>
+                )}
+              </Fragment>
+            ))}
+          </>
+        )}
       </DataGridRow>
     );
   },

From 0af77d086a05d584df5dcd02bcb817f8a6911ddd Mon Sep 17 00:00:00 2001
From: Robert <robertbrignull@github.com>
Date: Thu, 19 Oct 2023 12:52:45 +0100
Subject: [PATCH 3/5] Display validation errors in model editor

---
 .../src/view/model-editor/MethodRow.tsx          | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/extensions/ql-vscode/src/view/model-editor/MethodRow.tsx b/extensions/ql-vscode/src/view/model-editor/MethodRow.tsx
index a304920cc5d..13c50fdcdc9 100644
--- a/extensions/ql-vscode/src/view/model-editor/MethodRow.tsx
+++ b/extensions/ql-vscode/src/view/model-editor/MethodRow.tsx
@@ -31,6 +31,8 @@ import { ModelEditorViewState } from "../../model-editor/shared/view-state";
 import { Codicon } from "../common";
 import { canAddNewModeledMethod } from "../../model-editor/shared/multiple-modeled-methods";
 import { DataGridCell, DataGridRow } from "../common/DataGrid";
+import { validateModeledMethods } from "../../model-editor/shared/validation";
+import { ModeledMethodAlert } from "../method-modeling/ModeledMethodAlert";
 
 const ApiOrMethodRow = styled.div`
   min-height: calc(var(--input-height) * 1px);
@@ -111,6 +113,11 @@ const ModelableMethodRow = forwardRef<HTMLElement | undefined, MethodRowProps>(
       [modeledMethodsProp, method, viewState],
     );
 
+    const validationErrors = useMemo(
+      () => validateModeledMethods(modeledMethods),
+      [modeledMethods],
+    );
+
     const modeledMethodChangedHandlers = useMemo(
       () =>
         modeledMethods.map((_, index) => (modeledMethod: ModeledMethod) => {
@@ -163,7 +170,9 @@ const ModelableMethodRow = forwardRef<HTMLElement | undefined, MethodRowProps>(
         ref={ref}
         focused={revealedMethodSignature === method.signature}
       >
-        <DataGridCell gridRow={`span ${modeledMethods.length}`}>
+        <DataGridCell
+          gridRow={`span ${modeledMethods.length + validationErrors.length}`}
+        >
           <ApiOrMethodRow>
             <ModelingStatusIndicator status={modelingStatus} />
             <MethodClassifications method={method} />
@@ -256,6 +265,11 @@ const ModelableMethodRow = forwardRef<HTMLElement | undefined, MethodRowProps>(
                 )}
               </Fragment>
             ))}
+            {validationErrors.map((error, index) => (
+              <DataGridCell gridColumn="span 5" key={index}>
+                <ModeledMethodAlert error={error} />
+              </DataGridCell>
+            ))}
           </>
         )}
       </DataGridRow>

From 553435d5b76c3929636519a6b0df5a5502ab0eb0 Mon Sep 17 00:00:00 2001
From: Robert <robertbrignull@github.com>
Date: Thu, 19 Oct 2023 16:14:41 +0100
Subject: [PATCH 4/5] Add stories for validation errors in MethodRow

---
 .../model-editor/MethodRow.stories.tsx        | 25 +++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/extensions/ql-vscode/src/stories/model-editor/MethodRow.stories.tsx b/extensions/ql-vscode/src/stories/model-editor/MethodRow.stories.tsx
index 2269ba90b89..f6d66f688fb 100644
--- a/extensions/ql-vscode/src/stories/model-editor/MethodRow.stories.tsx
+++ b/extensions/ql-vscode/src/stories/model-editor/MethodRow.stories.tsx
@@ -153,3 +153,28 @@ MultipleModelings.args = {
   methodCanBeModeled: true,
   viewState,
 };
+
+export const ValidationError = Template.bind({});
+ValidationError.args = {
+  method,
+  modeledMethods: [
+    { ...modeledMethod, type: "source" },
+    { ...modeledMethod, type: "source" },
+  ],
+  methodCanBeModeled: true,
+  viewState,
+};
+
+export const MultipleValidationErrors = Template.bind({});
+MultipleValidationErrors.args = {
+  method,
+  modeledMethods: [
+    { ...modeledMethod, type: "source" },
+    { ...modeledMethod, type: "source" },
+    { ...modeledMethod, type: "sink" },
+    { ...modeledMethod, type: "sink" },
+    { ...modeledMethod, type: "neutral", kind: "source" },
+  ],
+  methodCanBeModeled: true,
+  viewState,
+};

From 31d654d33d84a6d9fe6d133c3de4539a3549ce34 Mon Sep 17 00:00:00 2001
From: Robert <robertbrignull@github.com>
Date: Thu, 19 Oct 2023 16:26:08 +0100
Subject: [PATCH 5/5] Add tests of showing validation errors

---
 .../model-editor/__tests__/MethodRow.spec.tsx | 58 +++++++++++++++++++
 1 file changed, 58 insertions(+)

diff --git a/extensions/ql-vscode/src/view/model-editor/__tests__/MethodRow.spec.tsx b/extensions/ql-vscode/src/view/model-editor/__tests__/MethodRow.spec.tsx
index bd690ce8cff..d798b291fd9 100644
--- a/extensions/ql-vscode/src/view/model-editor/__tests__/MethodRow.spec.tsx
+++ b/extensions/ql-vscode/src/view/model-editor/__tests__/MethodRow.spec.tsx
@@ -438,4 +438,62 @@ describe(MethodRow.name, () => {
       { ...modeledMethod, type: "summary" },
     ]);
   });
+
+  it("does not display validation errors when everything is valid", () => {
+    render({
+      modeledMethods: [
+        { ...modeledMethod, type: "source" },
+        { ...modeledMethod, type: "sink" },
+      ],
+      viewState: {
+        ...viewState,
+        showMultipleModels: true,
+      },
+    });
+
+    expect(screen.queryByRole("alert")).not.toBeInTheDocument();
+  });
+
+  it("displays a single validation error", () => {
+    render({
+      modeledMethods: [
+        { ...modeledMethod, type: "source" },
+        { ...modeledMethod, type: "source" },
+      ],
+      viewState: {
+        ...viewState,
+        showMultipleModels: true,
+      },
+    });
+
+    expect(screen.getByRole("alert")).toBeInTheDocument();
+    expect(
+      screen.getByText("Error: Duplicated classification"),
+    ).toBeInTheDocument();
+    expect(
+      screen.queryByText("Error: Conflicting classification"),
+    ).not.toBeInTheDocument();
+  });
+
+  it("displays multiple validation errors", () => {
+    render({
+      modeledMethods: [
+        { ...modeledMethod, type: "source" },
+        { ...modeledMethod, type: "source" },
+        { ...modeledMethod, type: "neutral", kind: "source" },
+      ],
+      viewState: {
+        ...viewState,
+        showMultipleModels: true,
+      },
+    });
+
+    expect(screen.getAllByRole("alert").length).toBe(2);
+    expect(
+      screen.getByText("Error: Duplicated classification"),
+    ).toBeInTheDocument();
+    expect(
+      screen.getByText("Error: Conflicting classification"),
+    ).toBeInTheDocument();
+  });
 });