Skip to content

Commit 4ac2123

Browse files
committed
Fix neutral model validation to consider kind
This fixes a bug where the validation of modeled methods would not consider the kind of the modeled method, and would therefore give an error when there was e.g. a neutral sink and a non-neutral summary.
1 parent 8ecc31f commit 4ac2123

2 files changed

Lines changed: 50 additions & 13 deletions

File tree

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

Lines changed: 28 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ function canonicalizeModeledMethod(
7575
type: "neutral",
7676
input: "",
7777
output: "",
78-
kind: "",
78+
kind: modeledMethod.kind,
7979
provenance: "manual",
8080
};
8181
default:
@@ -117,24 +117,40 @@ export function validateModeledMethods(
117117
}
118118
}
119119

120-
const neutralModeledMethod = consideredModeledMethods.find(
120+
const neutralModeledMethods = consideredModeledMethods.filter(
121121
(modeledMethod) => modeledMethod.type === "neutral",
122122
);
123-
const hasNonNeutralModeledMethod = consideredModeledMethods.some(
124-
(modeledMethod) => modeledMethod.type !== "neutral",
125-
);
126123

127-
// If there is a neutral model and any other model, that is an error
128-
if (neutralModeledMethod && hasNonNeutralModeledMethod) {
129-
// Another validation will validate that only one neutral method is present, so we only need
130-
// to return an error for the first one
124+
const neutralModeledMethodsByKind = new Map<string, ModeledMethod[]>();
125+
for (const neutralModeledMethod of neutralModeledMethods) {
126+
if (!neutralModeledMethodsByKind.has(neutralModeledMethod.kind)) {
127+
neutralModeledMethodsByKind.set(neutralModeledMethod.kind, []);
128+
}
129+
130+
neutralModeledMethodsByKind
131+
.get(neutralModeledMethod.kind)
132+
?.push(neutralModeledMethod);
133+
}
134+
135+
for (const [
136+
neutralModeledMethodKind,
137+
neutralModeledMethods,
138+
] of neutralModeledMethodsByKind) {
139+
const conflictingMethods = consideredModeledMethods.filter(
140+
(method) => neutralModeledMethodKind === method.type,
141+
);
142+
143+
if (conflictingMethods.length < 1) {
144+
continue;
145+
}
131146

132147
result.push({
133148
title: "Conflicting classification",
134-
message:
135-
"This method has a neutral classification, which conflicts with other classifications.",
149+
message: `This method has a neutral ${neutralModeledMethodKind} classification, which conflicts with other ${neutralModeledMethodKind} classifications.`,
136150
actionText: "Modify or remove the neutral classification.",
137-
index: modeledMethods.indexOf(neutralModeledMethod),
151+
// Another validation will validate that only one neutral method is present, so we only need
152+
// to return an error for the first one
153+
index: modeledMethods.indexOf(neutralModeledMethods[0]),
138154
});
139155
}
140156

extensions/ql-vscode/test/unit-tests/model-editor/shared/validation.test.ts

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -246,6 +246,7 @@ describe(validateModeledMethods.name, () => {
246246
}),
247247
createModeledMethod({
248248
type: "neutral",
249+
kind: "sink",
249250
}),
250251
];
251252

@@ -261,16 +262,34 @@ describe(validateModeledMethods.name, () => {
261262
]);
262263
});
263264

265+
it("should not give an error with other neutral combined with other models", () => {
266+
const modeledMethods = [
267+
createModeledMethod({
268+
type: "sink",
269+
}),
270+
createModeledMethod({
271+
type: "neutral",
272+
kind: "summary",
273+
}),
274+
];
275+
276+
const errors = validateModeledMethods(modeledMethods);
277+
278+
expect(errors).toEqual([]);
279+
});
280+
264281
it("should give an error with duplicate neutral combined with other models", () => {
265282
const modeledMethods = [
266283
createModeledMethod({
267284
type: "neutral",
285+
kind: "summary",
268286
}),
269287
createModeledMethod({
270-
type: "sink",
288+
type: "summary",
271289
}),
272290
createModeledMethod({
273291
type: "neutral",
292+
kind: "summary",
274293
}),
275294
];
276295

@@ -299,12 +318,14 @@ describe(validateModeledMethods.name, () => {
299318
}),
300319
createModeledMethod({
301320
type: "neutral",
321+
kind: "sink",
302322
}),
303323
createModeledMethod({
304324
type: "sink",
305325
}),
306326
createModeledMethod({
307327
type: "neutral",
328+
kind: "sink",
308329
}),
309330
];
310331

0 commit comments

Comments
 (0)