Validate `Fix` objects provided by Checks Plugins

Release-Notes: skip
Google-Bug-Id: b/249804364
Change-Id: I5a370e0e39234c88297a45e996aaabb218301883

2 files changed, 95 insertions, 10 deletions

diff --git a/polygerrit-ui/app/models/checks/checks-util.ts b/polygerrit-ui/app/models/checks/checks-util.ts
index 09c9273445..7ccdf9183a 100644
--- a/polygerrit-ui/app/models/checks/checks-util.ts
+++ b/polygerrit-ui/app/models/checks/checks-util.ts
@@ -8,12 +8,16 @@ import {
   Category,
   CheckResult as CheckResultApi,
   CheckRun as CheckRunApi,
+  Fix,
   Link,
   LinkIcon,
+  Replacement,
   RunStatus,
 } from '../../api/checks';
 import {PatchSetNumber} from '../../api/rest-api';
+import {FixSuggestionInfo, FixReplacementInfo} from '../../types/common';
 import {OpenFixPreviewEventDetail} from '../../types/events';
+import {notUndefined} from '../../types/types';
 import {PROVIDED_FIX_ID} from '../../utils/comment-util';
 import {assert, assertNever} from '../../utils/common-util';
 import {fire} from '../../utils/event-util';
@@ -86,17 +90,15 @@ export function createFixAction(
   target: EventTarget,
   result?: RunResult
 ): Action | undefined {
-  const fixes = result?.fixes;
-  if (!fixes || fixes?.length === 0 || !result?.patchset) return;
+  if (!result?.patchset) return;
+  if (!result?.fixes) return;
+  const fixSuggestions = result.fixes
+    .map(f => rectifyFix(f, result?.checkName))
+    .filter(notUndefined);
+  if (fixSuggestions.length === 0) return;
   const eventDetail: OpenFixPreviewEventDetail = {
-    patchNum: result?.patchset as PatchSetNumber,
-    fixSuggestions: fixes.map(fix => {
-      return {
-        description: `Fix provided by ${result?.checkName}`,
-        fix_id: PROVIDED_FIX_ID,
-        ...fix,
-      };
-    }),
+    patchNum: result.patchset as PatchSetNumber,
+    fixSuggestions,
   };
   return {
     name: 'Show Fix',
@@ -107,6 +109,36 @@ export function createFixAction(
   };
 }
 
+export function rectifyFix(
+  fix: Fix | undefined,
+  checkName: string
+): FixSuggestionInfo | undefined {
+  if (!fix?.replacements) return undefined;
+  const replacements = fix.replacements
+    .map(rectifyReplacement)
+    .filter(notUndefined);
+  if (replacements.length === 0) return undefined;
+
+  return {
+    description: fix.description ?? `Fix provided by ${checkName}`,
+    fix_id: PROVIDED_FIX_ID,
+    replacements,
+  };
+}
+
+export function rectifyReplacement(
+  r: Replacement | undefined
+): FixReplacementInfo | undefined {
+  if (!r?.path) return undefined;
+  if (!r?.range) return undefined;
+  if (r?.replacement === undefined) return undefined;
+  if (!Number.isInteger(r.range.start_line)) return undefined;
+  if (!Number.isInteger(r.range.end_line)) return undefined;
+  if (!Number.isInteger(r.range.start_character)) return undefined;
+  if (!Number.isInteger(r.range.end_character)) return undefined;
+  return r;
+}
+
 export function worstCategory(run: CheckRun) {
   if (hasResultsOf(run, Category.ERROR)) return Category.ERROR;
   if (hasResultsOf(run, Category.WARNING)) return Category.WARNING;
diff --git a/polygerrit-ui/app/models/checks/checks-util_test.ts b/polygerrit-ui/app/models/checks/checks-util_test.ts
index 5a7bd75545..c237c59b53 100644
--- a/polygerrit-ui/app/models/checks/checks-util_test.ts
+++ b/polygerrit-ui/app/models/checks/checks-util_test.ts
@@ -10,9 +10,13 @@ import {
   ALL_ATTEMPTS,
   AttemptChoice,
   LATEST_ATTEMPT,
+  rectifyFix,
   sortAttemptChoices,
   stringToAttemptChoice,
 } from './checks-util';
+import {Fix, Replacement} from '../../api/checks';
+import {CommentRange} from '../../api/core';
+import {PROVIDED_FIX_ID} from '../../utils/comment-util';
 
 suite('checks-util tests', () => {
   setup(() => {});
@@ -33,6 +37,55 @@ suite('checks-util tests', () => {
     assert.equal(stringToAttemptChoice('1x'), undefined);
   });
 
+  test('rectifyFix', () => {
+    assert.isUndefined(rectifyFix(undefined, 'name'));
+    assert.isUndefined(rectifyFix({} as Fix, 'name'));
+    assert.isUndefined(
+      rectifyFix({description: 'asdf', replacements: []}, 'name')
+    );
+    assert.isUndefined(
+      rectifyFix(
+        {description: 'asdf', replacements: [{} as Replacement]},
+        'test-check-name'
+      )
+    );
+    assert.isUndefined(
+      rectifyFix(
+        {
+          description: 'asdf',
+          replacements: [
+            {
+              path: 'test-path',
+              range: {} as CommentRange,
+              replacement: 'test-replacement-string',
+            },
+          ],
+        },
+        'test-check-name'
+      )
+    );
+    const rectified = rectifyFix(
+      {
+        replacements: [
+          {
+            path: 'test-path',
+            range: {
+              start_line: 1,
+              end_line: 1,
+              start_character: 0,
+              end_character: 1,
+            } as CommentRange,
+            replacement: 'test-replacement-string',
+          },
+        ],
+      },
+      'test-check-name'
+    );
+    assert.isDefined(rectified);
+    assert.equal(rectified?.description, 'Fix provided by test-check-name');
+    assert.equal(rectified?.fix_id, PROVIDED_FIX_ID);
+  });
+
   test('sortAttemptChoices', () => {
     const unsorted: (AttemptChoice | undefined)[] = [
       3,