fix(amazonq): auto-review removes existing issues

packages/amazonq/.changes/next-release/Bug Fix-c8a60497-e42c-4141-bcfe-2c44188bc98b.json

@@ -0,0 +1,4 @@
+{
+	"type": "Bug Fix",
+	"description": "/review: Auto-review should not remove issues from manual reviews"
+}

packages/core/src/codewhisperer/activation.ts

@@ -614,7 +614,6 @@ function toggleIssuesVisibility(visibleCondition: (issue: CodeScanIssue, filePat
         ...group,
         issues: group.issues.map((issue) => ({ ...issue, visible: visibleCondition(issue, group.filePath) })),
     }))
-    securityScanRender.securityDiagnosticCollection?.clear()
     for (const issue of updatedIssues) {
         updateSecurityDiagnosticCollection(issue)
     }

packages/core/src/codewhisperer/service/diagnosticsProvider.ts

@@ -4,7 +4,7 @@
  */
 
 import * as vscode from 'vscode'
-import { CodeScanIssue, AggregatedCodeScanIssue, CodeScansState } from '../models/model'
+import { CodeScanIssue, AggregatedCodeScanIssue } from '../models/model'
 import { CodeAnalysisScope, codewhispererDiagnosticSourceLabel } from '../models/constants'
 import { SecurityIssueTreeViewProvider } from './securityIssueTreeViewProvider'
 import { SecurityIssueProvider } from './securityIssueProvider'
@@ -30,24 +30,30 @@ export function initSecurityScanRender(
     scope: CodeAnalysisScope
 ) {
     securityScanRender.initialized = false
-    if ((scope === CodeAnalysisScope.FILE_AUTO || scope === CodeAnalysisScope.FILE_ON_DEMAND) && editor) {
+    if (scope === CodeAnalysisScope.FILE_ON_DEMAND && editor) {
         securityScanRender.securityDiagnosticCollection?.delete(editor.document.uri)
     } else if (scope === CodeAnalysisScope.PROJECT) {
         securityScanRender.securityDiagnosticCollection?.clear()
     }
     for (const securityRecommendation of securityRecommendationList) {
         updateSecurityDiagnosticCollection(securityRecommendation)
-        updateSecurityIssuesForProviders(securityRecommendation)
+        updateSecurityIssuesForProviders(securityRecommendation, scope === CodeAnalysisScope.FILE_AUTO)
     }
     securityScanRender.initialized = true
 }
 
-function updateSecurityIssuesForProviders(securityRecommendation: AggregatedCodeScanIssue) {
-    const updatedSecurityRecommendationList = [
-        ...SecurityIssueProvider.instance.issues.filter((group) => group.filePath !== securityRecommendation.filePath),
-        securityRecommendation,
-    ]
-    SecurityIssueProvider.instance.issues = updatedSecurityRecommendationList
+function updateSecurityIssuesForProviders(securityRecommendation: AggregatedCodeScanIssue, isAutoScope?: boolean) {
+    if (isAutoScope) {
+        SecurityIssueProvider.instance.mergeIssues(securityRecommendation)
+    } else {
+        const updatedSecurityRecommendationList = [
+            ...SecurityIssueProvider.instance.issues.filter(
+                (group) => group.filePath !== securityRecommendation.filePath
+            ),
+            securityRecommendation,
+        ]
+        SecurityIssueProvider.instance.issues = updatedSecurityRecommendationList
+    }
     SecurityIssueTreeViewProvider.instance.refresh()
 }
 
@@ -58,8 +64,22 @@ export function updateSecurityDiagnosticCollection(securityRecommendation: Aggre
     const securityDiagnostics: vscode.Diagnostic[] = vscode.languages
         .getDiagnostics(uri)
         .filter((diagnostic) => diagnostic.source === codewhispererDiagnosticSourceLabel)
-    for (const securityIssue of securityRecommendation.issues.filter((securityIssue) => securityIssue.visible)) {
-        securityDiagnostics.push(createSecurityDiagnostic(securityIssue))
+    for (const securityIssue of securityRecommendation.issues) {
+        const existingDiagnosticIndex = securityDiagnostics.findIndex(
+            (diagnostic) =>
+                (diagnostic.message === securityIssue.title &&
+                    diagnostic.range.start.line === securityIssue.startLine &&
+                    diagnostic.range.end.line === securityIssue.endLine) ||
+                (diagnostic.message === 'Re-scan to validate the fix: ' + securityIssue.title &&
+                    diagnostic.range.start.line === securityIssue.startLine &&
+                    diagnostic.range.end.line === securityIssue.startLine)
+        )
+        if (existingDiagnosticIndex !== -1) {
+            securityDiagnostics.splice(existingDiagnosticIndex, 1)
+        }
+        if (securityIssue.visible) {
+            securityDiagnostics.push(createSecurityDiagnostic(securityIssue))
+        }
     }
     securityDiagnosticCollection.set(uri, securityDiagnostics)
 }
@@ -112,8 +132,7 @@ export function disposeSecurityDiagnostic(event: vscode.TextDocumentChangeEvent)
             if (
                 issue.severity === vscode.DiagnosticSeverity.Warning &&
                 intersection &&
-                (/\S/.test(changedText) || changedText === '') &&
-                !CodeScansState.instance.isScansEnabled()
+                (/\S/.test(changedText) || changedText === '')
             ) {
                 issue.severity = vscode.DiagnosticSeverity.Information
                 issue.message = 'Re-scan to validate the fix: ' + issue.message

packages/core/src/codewhisperer/service/securityIssueProvider.ts

@@ -4,7 +4,7 @@
  */
 
 import * as vscode from 'vscode'
-import { AggregatedCodeScanIssue, CodeScanIssue, CodeScansState, SuggestedFix } from '../models/model'
+import { AggregatedCodeScanIssue, CodeScanIssue, SuggestedFix } from '../models/model'
 export class SecurityIssueProvider {
     static #instance: SecurityIssueProvider
     public static get instance() {
@@ -53,11 +53,7 @@ export class SecurityIssueProvider {
                             event.document.lineAt(issue.endLine - 1)?.range.end.character ?? 0
                         )
                         const intersection = changedRange.intersection(range)
-                        return !(
-                            intersection &&
-                            (/\S/.test(changedText) || changedText === '') &&
-                            !CodeScansState.instance.isScansEnabled()
-                        )
+                        return !(intersection && (/\S/.test(changedText) || changedText === ''))
                     })
                     .map((issue) => {
                         if (issue.startLine < changedRange.end.line) {
@@ -132,4 +128,32 @@ export class SecurityIssueProvider {
             }
         })
     }
+
+    public mergeIssues(newIssues: AggregatedCodeScanIssue) {
+        const existingGroup = this._issues.find((group) => group.filePath === newIssues.filePath)
+        if (!existingGroup) {
+            this._issues.push(newIssues)
+            return
+        }
+
+        this._issues = this._issues.map((group) =>
+            group.filePath !== newIssues.filePath
+                ? group
+                : {
+                      ...group,
+                      issues: [
+                          ...group.issues,
+                          ...newIssues.issues.filter((issue) => !this.isExistingIssue(issue, newIssues.filePath)),
+                      ],
+                  }
+        )
+    }
+
+    private isExistingIssue(issue: CodeScanIssue, filePath: string) {
+        return this._issues
+            .find((group) => group.filePath === filePath)
+            ?.issues.find(
+                (i) => i.title === issue.title && i.startLine === issue.startLine && i.endLine === issue.endLine
+            )
+    }
 }

packages/core/src/shared/utilities/commentUtils.ts

@@ -90,13 +90,16 @@ export function insertCommentAboveLine(document: vscode.TextDocument, line: numb
     }
 
     const edit = new vscode.WorkspaceEdit()
-    const position = new vscode.Position(line, 0)
+    const previousLine = Math.max(0, line - 1)
+    const previousLineLength = previousLine > 0 ? document.lineAt(previousLine).text.length : 0
+    const position = new vscode.Position(previousLine, previousLineLength)
     const indent = ' '.repeat(Math.max(0, document.lineAt(line).firstNonWhitespaceCharacterIndex))
+    const commentPrefix = line === 0 ? '' : '\n'
     const commentText = lineComment
-        ? `${indent}${lineComment} ${comment}\n`
+        ? `${commentPrefix}${indent}${lineComment} ${comment}`
         : blockComment?.[0] && blockComment[1]
-          ? `${indent}${blockComment[0]} ${comment} ${blockComment[1]}\n`
-          : `${indent}${defaultCommentConfig.lineComment} ${comment}\n`
+          ? `${commentPrefix}${indent}${blockComment[0]} ${comment} ${blockComment[1]}`
+          : `${commentPrefix}${indent}${defaultCommentConfig.lineComment} ${comment}`
     edit.insert(document.uri, position, commentText)
     void vscode.workspace.applyEdit(edit)
 }

packages/core/src/test/shared/utilities/commentUtils.test.ts

@@ -77,7 +77,7 @@ describe('CommentUtils', function () {
 
             const document = createMockDocument('foo = 1\nbar = 2', 'foo.py', 'python')
             insertCommentAboveLine(document, 1, 'some-comment')
-            assert.ok(insertMock.calledOnceWith(document.uri, new vscode.Position(1, 0), '# some-comment\n'))
+            assert.ok(insertMock.calledOnceWith(document.uri, new vscode.Position(0, 0), '\n# some-comment'))
         })
 
         it('should indent the comment by the same amount as the current line', function () {
@@ -86,7 +86,7 @@ describe('CommentUtils', function () {
 
             const document = createMockDocument('    foo = 1\n    bar = 2', 'foo.py', 'python')
             insertCommentAboveLine(document, 1, 'some-comment')
-            assert.ok(insertMock.calledOnceWith(document.uri, new vscode.Position(1, 0), '    # some-comment\n'))
+            assert.ok(insertMock.calledOnceWith(document.uri, new vscode.Position(0, 0), '\n    # some-comment'))
         })
 
         it('should fallback to block comment if line comment is undefined', function () {
@@ -95,7 +95,7 @@ describe('CommentUtils', function () {
 
             const document = createMockDocument('<aaa>\n  <bbb></bbb>\n</aaa>', 'foo.xml', 'xml')
             insertCommentAboveLine(document, 1, 'some-comment')
-            assert.ok(insertMock.calledOnceWith(document.uri, new vscode.Position(1, 0), '  <!-- some-comment -->\n'))
+            assert.ok(insertMock.calledOnceWith(document.uri, new vscode.Position(0, 0), '\n  <!-- some-comment -->'))
         })
     })
 })