feat(amazonq): grouping options for code issues

Author: ctlai95

packages/amazonq/.changes/next-release/Feature-f0d4f2ca-72c5-4434-8cf3-fcd742bf33d7.json

@@ -0,0 +1,4 @@
+{
+	"type": "Feature",
+	"description": "/review: Code issues can be grouped by file location or severity"
+}

packages/amazonq/package.json

@@ -370,10 +370,15 @@
                     "when": "view == aws.AmazonQChatView || view == aws.amazonq.AmazonCommonAuth",
                     "group": "y_toolkitMeta@2"
                 },
+                {
+                    "command": "aws.amazonq.codescan.showGroupingStrategy",
+                    "when": "view == aws.amazonq.SecurityIssuesTree",
+                    "group": "navigation@1"
+                },
                 {
                     "command": "aws.amazonq.security.showFilters",
                     "when": "view == aws.amazonq.SecurityIssuesTree",
-                    "group": "navigation"
+                    "group": "navigation@2"
                 }
             ],
             "view/item/context": [
@@ -729,6 +734,12 @@
             {
                 "command": "aws.amazonq.security.showFilters",
                 "title": "%AWS.command.amazonq.filterIssues%",
+                "icon": "$(filter)",
+                "enablement": "view == aws.amazonq.SecurityIssuesTree"
+            },
+            {
+                "command": "aws.amazonq.codescan.showGroupingStrategy",
+                "title": "%AWS.command.amazonq.groupIssues%",
                 "icon": "$(list-filter)",
                 "enablement": "view == aws.amazonq.SecurityIssuesTree"
             },

packages/amazonq/test/unit/codewhisperer/service/securityIssueTreeViewProvider.test.ts

@@ -10,10 +10,13 @@ import {
     SecurityTreeViewFilterState,
     SecurityIssueProvider,
     SeverityItem,
+    CodeIssueGroupingStrategyState,
+    CodeIssueGroupingStrategy,
 } from 'aws-core-vscode/codewhisperer'
 import { createCodeScanIssue } from 'aws-core-vscode/test'
 import assert from 'assert'
 import sinon from 'sinon'
+import path from 'path'
 
 describe('SecurityIssueTreeViewProvider', function () {
     let securityIssueProvider: SecurityIssueProvider
@@ -102,5 +105,88 @@ describe('SecurityIssueTreeViewProvider', function () {
             const result = securityIssueTreeViewProvider.getChildren(element) as IssueItem[]
             assert.strictEqual(result.length, 0)
         })
+
+        it('should return severity-grouped items when grouping strategy is Severity', function () {
+            sinon.stub(CodeIssueGroupingStrategyState.instance, 'getState').returns(CodeIssueGroupingStrategy.Severity)
+            securityIssueProvider.issues = [
+                {
+                    filePath: 'file/path/c',
+                    issues: [
+                        createCodeScanIssue({ severity: 'Critical' }),
+                        createCodeScanIssue({ severity: 'Critical' }),
+                    ],
+                },
+                {
+                    filePath: 'file/path/d',
+                    issues: [createCodeScanIssue({ severity: 'Critical' })],
+                },
+                {
+                    filePath: 'file/path/a',
+                    issues: [createCodeScanIssue({ severity: 'Critical' }), createCodeScanIssue({ severity: 'High' })],
+                },
+            ]
+
+            const severityItems = securityIssueTreeViewProvider.getChildren() as SeverityItem[]
+            for (const [index, [severity, expectedIssueCount]] of [
+                ['Critical', 4],
+                ['High', 1],
+                ['Medium', 0],
+                ['Low', 0],
+                ['Info', 0],
+            ].entries()) {
+                const currentSeverityItem = severityItems[index]
+                assert.strictEqual(currentSeverityItem.label, severity)
+                assert.strictEqual(currentSeverityItem.issues.length, expectedIssueCount)
+
+                const issueItems = securityIssueTreeViewProvider.getChildren(currentSeverityItem) as IssueItem[]
+                assert.ok(issueItems.every((item) => item.iconPath === undefined))
+                assert.ok(
+                    issueItems.every((item) => item.description?.toString().startsWith(path.basename(item.filePath)))
+                )
+            }
+        })
+
+        it('should return file-grouped items when grouping strategy is FileLocation', function () {
+            sinon
+                .stub(CodeIssueGroupingStrategyState.instance, 'getState')
+                .returns(CodeIssueGroupingStrategy.FileLocation)
+            securityIssueProvider.issues = [
+                {
+                    filePath: 'file/path/c',
+                    issues: [
+                        createCodeScanIssue({ severity: 'Critical' }),
+                        createCodeScanIssue({ severity: 'Critical' }),
+                    ],
+                },
+                {
+                    filePath: 'file/path/d',
+                    issues: [createCodeScanIssue({ severity: 'Critical' })],
+                },
+                {
+                    filePath: 'file/path/a',
+                    issues: [createCodeScanIssue({ severity: 'Critical' }), createCodeScanIssue({ severity: 'High' })],
+                },
+            ]
+
+            const result = securityIssueTreeViewProvider.getChildren() as FileItem[]
+            for (const [index, [fileName, expectedIssueCount]] of [
+                ['a', 2],
+                ['c', 2],
+                ['d', 1],
+            ].entries()) {
+                const currentFileItem = result[index]
+                assert.strictEqual(currentFileItem.label, fileName)
+                assert.strictEqual(currentFileItem.issues.length, expectedIssueCount)
+                assert.strictEqual(currentFileItem.description, 'file/path')
+
+                const issueItems = securityIssueTreeViewProvider.getChildren(currentFileItem) as IssueItem[]
+                assert.ok(
+                    issueItems.every((item) =>
+                        item.iconPath?.toString().includes(`${item.issue.severity.toLowerCase()}.svg`)
+                    )
+                )
+                assert.ok(issueItems.every((item) => item.description?.toString().startsWith('[Ln ')))
+            }
+        })
     })
 })

packages/core/package.nls.json

@@ -133,6 +133,7 @@
     "AWS.command.amazonq.acceptFix": "Accept Fix",
     "AWS.command.amazonq.regenerateFix": "Regenerate Fix",
     "AWS.command.amazonq.filterIssues": "Filter Issues",
+    "AWS.command.amazonq.groupIssues": "Group Issues",
     "AWS.command.deploySamApplication": "Deploy SAM Application",
     "AWS.command.aboutToolkit": "About",
     "AWS.command.downloadLambda": "Download...",
@@ -309,6 +310,8 @@
     "AWS.amazonq.scans.projectScanInProgress": "Workspace review is in progress...",
     "AWS.amazonq.scans.fileScanInProgress": "File review is in progress...",
     "AWS.amazonq.scans.noGitRepo": "Your workspace is not in a git repository. I'll review your project files for security issues, and your in-flight changes for code quality issues.",
+    "AWS.amazonq.scans.severity": "Severity",
+    "AWS.amazonq.scans.fileLocation": "File Location",
     "AWS.amazonq.featureDev.error.conversationIdNotFoundError": "Conversation id must exist before starting code generation",
     "AWS.amazonq.featureDev.error.contentLengthError": "The folder you selected is too large for me to use as context. Please choose a smaller folder to work on. For more information on quotas, see the <a href=\"https://docs.aws.amazon.com/amazonq/latest/qdeveloper-ug/software-dev.html#quotas\" target=\"_blank\">Amazon Q Developer documentation.</a>",
     "AWS.amazonq.featureDev.error.illegalStateTransition": "Illegal transition between states, restart the conversation",

packages/core/src/codewhisperer/activation.ts

@@ -18,6 +18,7 @@ import {
     SecurityTreeViewFilterState,
     AggregatedCodeScanIssue,
     CodeScanIssue,
+    CodeIssueGroupingStrategyState,
 } from './models/model'
 import { invokeRecommendation } from './commands/invokeRecommendation'
 import { acceptSuggestion } from './commands/onInlineAcceptance'
@@ -60,6 +61,7 @@ import {
     ignoreAllIssues,
     focusIssue,
     showExploreAgentsView,
+    showCodeIssueGroupingQuickPick,
 } from './commands/basicCommands'
 import { sleep } from '../shared/utilities/timeoutUtils'
 import { ReferenceLogViewProvider } from './service/referenceLogViewProvider'
@@ -288,6 +290,8 @@ export async function activate(context: ExtContext): Promise<void> {
         listCodeWhispererCommands.register(),
         // quick pick with security issues tree filters
         showSecurityIssueFilters.register(),
+        // quick pick code issue grouping strategy
+        showCodeIssueGroupingQuickPick.register(),
         // reset security issue filters
         clearFilters.register(),
         // handle security issues tree item clicked
@@ -296,6 +300,10 @@ export async function activate(context: ExtContext): Promise<void> {
         SecurityTreeViewFilterState.instance.onDidChangeState((e) => {
             SecurityIssueTreeViewProvider.instance.refresh()
         }),
+        // refresh treeview when grouping strategy changes
+        CodeIssueGroupingStrategyState.instance.onDidChangeState((e) => {
+            SecurityIssueTreeViewProvider.instance.refresh()
+        }),
         // show a no match state
         SecurityIssueTreeViewProvider.instance.onDidChangeTreeData((e) => {
             const noMatches =

packages/core/src/codewhisperer/commands/basicCommands.ts

@@ -13,6 +13,9 @@ import { confirmStopSecurityScan, startSecurityScan } from './startSecurityScan'
 import { SecurityPanelViewProvider } from '../views/securityPanelViewProvider'
 import {
     codeFixState,
+    codeIssueGroupingStrategies,
+    codeIssueGroupingStrategyLabel,
+    CodeIssueGroupingStrategyState,
     CodeScanIssue,
     CodeScansState,
     codeScanState,
@@ -884,6 +887,33 @@ export const showSecurityIssueFilters = Commands.declare({ id: 'aws.amazonq.secu
     }
 })
 
+export const showCodeIssueGroupingQuickPick = Commands.declare(
+    { id: 'aws.amazonq.codescan.showGroupingStrategy' },
+    () => async () => {
+        const quickPick = vscode.window.createQuickPick()
+        quickPick.title = localize('aws.commands.amazonq.groupIssues', 'Group Issues')
+        quickPick.placeholder = localize(
+            'aws.amazonq.codescan.groupIssues.placeholder',
+            'Select how to group code issues'
+        )
+        quickPick.items = codeIssueGroupingStrategies.map((strategy) => ({
+            label: codeIssueGroupingStrategyLabel[strategy],
+        }))
+        const groupingStrategy = CodeIssueGroupingStrategyState.instance.getState()
+        quickPick.activeItems = quickPick.items.filter(
+            (item) => item.label === codeIssueGroupingStrategyLabel[groupingStrategy]
+        )
+        quickPick.show()
+        quickPick.onDidChangeSelection(async (items) => {
+            const [item] = items
+            await CodeIssueGroupingStrategyState.instance.setState(
+                Object.entries(codeIssueGroupingStrategyLabel).find(([, label]) => label === item.label)?.[0]
+            )
+            quickPick.hide()
+        })
+    }
+)
+
 export const focusIssue = Commands.declare(
     { id: 'aws.amazonq.security.focusIssue' },
     () => async (issue: CodeScanIssue, filePath: string) => {

packages/core/src/codewhisperer/models/model.ts

@@ -20,6 +20,7 @@ import { TransformationSteps } from '../client/codewhispereruserclient'
 import { Messenger } from '../../amazonqGumby/chat/controller/messenger/messenger'
 import { TestChatControllerEventEmitters } from '../../amazonqTest/chat/controller/controller'
 import { ScanChatControllerEventEmitters } from '../../amazonqScan/controller'
+import { localize } from '../../shared/utilities/vsCodeUtils'
 
 // unavoidable global variables
 interface VsCodeState {
@@ -564,6 +565,52 @@ export class SecurityTreeViewFilterState {
     }
 }
 
+export enum CodeIssueGroupingStrategy {
+    Severity = 'Severity',
+    FileLocation = 'FileLocation',
+}
+const defaultCodeIssueGroupingStrategy = CodeIssueGroupingStrategy.Severity
+
+export const codeIssueGroupingStrategies = Object.values(CodeIssueGroupingStrategy)
+export const codeIssueGroupingStrategyLabel: Record<CodeIssueGroupingStrategy, string> = {
+    [CodeIssueGroupingStrategy.Severity]: localize('AWS.amazonq.scans.severity', 'Severity'),
+    [CodeIssueGroupingStrategy.FileLocation]: localize('AWS.amazonq.scans.fileLocation', 'File Location'),
+}
+
+export class CodeIssueGroupingStrategyState {
+    #fallback: CodeIssueGroupingStrategy
+    #onDidChangeState = new vscode.EventEmitter<CodeIssueGroupingStrategy>()
+    onDidChangeState = this.#onDidChangeState.event
+
+    static #instance: CodeIssueGroupingStrategyState
+    static get instance() {
+        return (this.#instance ??= new this())
+    }
+
+    protected constructor(fallback: CodeIssueGroupingStrategy = defaultCodeIssueGroupingStrategy) {
+        this.#fallback = fallback
+    }
+
+    public getState(): CodeIssueGroupingStrategy {
+        const state = globals.globalState.tryGet('aws.amazonq.codescan.groupingStrategy', String)
+        return this.isValidGroupingStrategy(state) ? state : this.#fallback
+    }
+
+    public async setState(_state: unknown) {
+        const state = this.isValidGroupingStrategy(_state) ? _state : this.#fallback
+        await globals.globalState.update('aws.amazonq.codescan.groupingStrategy', state)
+        this.#onDidChangeState.fire(state)
+    }
+
+    private isValidGroupingStrategy(strategy: unknown): strategy is CodeIssueGroupingStrategy {
+        return Object.values(CodeIssueGroupingStrategy).includes(strategy as CodeIssueGroupingStrategy)
+    }
+
+    public reset() {
+        return this.setState(this.#fallback)
+    }
+}
+
 /**
  *  Q - Transform
  */

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

@@ -4,7 +4,14 @@
  */
 import * as vscode from 'vscode'
 import path from 'path'
-import { CodeScanIssue, SecurityTreeViewFilterState, severities, Severity } from '../models/model'
+import {
+    CodeIssueGroupingStrategy,
+    CodeIssueGroupingStrategyState,
+    CodeScanIssue,
+    SecurityTreeViewFilterState,
+    severities,
+    Severity,
+} from '../models/model'
 import globals from '../../shared/extensionGlobals'
 import { getLogger } from '../../shared/logger'
 import { SecurityIssueProvider } from './securityIssueProvider'
@@ -34,6 +41,17 @@ export class SecurityIssueTreeViewProvider implements vscode.TreeDataProvider<Se
     }
 
     public getChildren(element?: SecurityViewTreeItem | undefined): vscode.ProviderResult<SecurityViewTreeItem[]> {
+        const groupingStrategy = CodeIssueGroupingStrategyState.instance.getState()
+        switch (groupingStrategy) {
+            case CodeIssueGroupingStrategy.FileLocation:
+                return this.getChildrenGroupedByFileLocation(element)
+            case CodeIssueGroupingStrategy.Severity:
+            default:
+                return this.getChildrenGroupedBySeverity(element)
+        }
+    }
+
+    private getChildrenGroupedBySeverity(element: SecurityViewTreeItem | undefined) {
         const filterHiddenSeverities = (severity: Severity) =>
             !SecurityTreeViewFilterState.instance.getHiddenSeverities().includes(severity)
 
@@ -64,6 +82,27 @@ export class SecurityIssueTreeViewProvider implements vscode.TreeDataProvider<Se
         return result
     }
 
+    private getChildrenGroupedByFileLocation(element: SecurityViewTreeItem | undefined) {
+        const filterHiddenSeverities = (issue: CodeScanIssue) =>
+            !SecurityTreeViewFilterState.instance.getHiddenSeverities().includes(issue.severity)
+
+        if (element instanceof FileItem) {
+            return element.issues
+                .filter(filterHiddenSeverities)
+                .filter((issue) => issue.visible)
+                .sort((a, b) => a.startLine - b.startLine)
+                .map((issue) => new IssueItem(element.filePath, issue))
+        }
+
+        const result = this.issueProvider.issues
+            .filter((group) => group.issues.some(filterHiddenSeverities))
+            .filter((group) => group.issues.some((issue) => issue.visible))
+            .sort((a, b) => a.filePath.localeCompare(b.filePath))
+            .map((group) => new FileItem(group.filePath, group.issues.filter(filterHiddenSeverities)))
+        this._onDidChangeTreeData.fire(result)
+        return result
+    }
+
     public refresh(): void {
         this._onDidChangeTreeData.fire()
     }
@@ -118,7 +157,8 @@ export class IssueItem extends vscode.TreeItem {
         public readonly issue: CodeScanIssue
     ) {
         super(issue.title, vscode.TreeItemCollapsibleState.None)
-        this.description = `${path.basename(this.filePath)} [Ln ${this.issue.startLine + 1}, Col 1]`
+        this.description = this.getDescription()
+        this.iconPath = this.getSeverityIcon()
         this.tooltip = this.getTooltipMarkdown()
         this.command = {
             title: 'Focus Issue',
@@ -132,6 +172,22 @@ export class IssueItem extends vscode.TreeItem {
         return globals.context.asAbsolutePath(`resources/images/severity-${this.issue.severity.toLowerCase()}.svg`)
     }
 
+    private getSeverityIcon() {
+        const iconPath = globals.context.asAbsolutePath(
+            `resources/icons/aws/amazonq/severity-${this.issue.severity.toLowerCase()}.svg`
+        )
+        const groupingStrategy = CodeIssueGroupingStrategyState.instance.getState()
+        return groupingStrategy !== CodeIssueGroupingStrategy.Severity ? iconPath : undefined
+    }
+
+    private getDescription() {
+        const positionStr = `[Ln ${this.issue.startLine + 1}, Col 1]`
+        const groupingStrategy = CodeIssueGroupingStrategyState.instance.getState()
+        return groupingStrategy !== CodeIssueGroupingStrategy.FileLocation
+            ? `${path.basename(this.filePath)} ${positionStr}`
+            : positionStr
+    }
+
     private getContextValue() {
         return this.issue.suggestedFixes.length === 0 || !this.issue.suggestedFixes[0].code
             ? ContextValue.ISSUE_WITHOUT_FIX

packages/core/src/shared/globalState.ts

@@ -32,6 +32,7 @@ export type globalKey =
     | 'aws.amazonq.hasShownWalkthrough'
     | 'aws.amazonq.showTryChatCodeLens'
     | 'aws.amazonq.securityIssueFilters'
+    | 'aws.amazonq.codescan.groupingStrategy'
     | 'aws.amazonq.notifications'
     | 'aws.amazonq.welcomeChatShowCount'
     | 'aws.amazonq.disclaimerAcknowledged'