Skip to content

Commit 69ae81e

Browse files
Merge master into feature/sdkv3
2 parents 343651e + 062d24a commit 69ae81e

File tree

32 files changed

+459
-76
lines changed

32 files changed

+459
-76
lines changed
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
{
2+
"type": "Feature",
3+
"description": "/review: Code issues can be grouped by file location or severity"
4+
}

packages/amazonq/package.json

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -365,10 +365,15 @@
365365
"when": "view == aws.AmazonQChatView || view == aws.amazonq.AmazonCommonAuth",
366366
"group": "y_toolkitMeta@2"
367367
},
368+
{
369+
"command": "aws.amazonq.codescan.showGroupingStrategy",
370+
"when": "view == aws.amazonq.SecurityIssuesTree",
371+
"group": "navigation@1"
372+
},
368373
{
369374
"command": "aws.amazonq.security.showFilters",
370375
"when": "view == aws.amazonq.SecurityIssuesTree",
371-
"group": "navigation"
376+
"group": "navigation@2"
372377
}
373378
],
374379
"view/item/context": [
@@ -724,6 +729,12 @@
724729
{
725730
"command": "aws.amazonq.security.showFilters",
726731
"title": "%AWS.command.amazonq.filterIssues%",
732+
"icon": "$(filter)",
733+
"enablement": "view == aws.amazonq.SecurityIssuesTree"
734+
},
735+
{
736+
"command": "aws.amazonq.codescan.showGroupingStrategy",
737+
"title": "%AWS.command.amazonq.groupIssues%",
727738
"icon": "$(list-filter)",
728739
"enablement": "view == aws.amazonq.SecurityIssuesTree"
729740
},

packages/amazonq/test/unit/codewhisperer/models/model.test.ts

Lines changed: 102 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,12 @@
44
*/
55
import assert from 'assert'
66
import sinon from 'sinon'
7-
import { SecurityIssueFilters, SecurityTreeViewFilterState } from 'aws-core-vscode/codewhisperer'
7+
import {
8+
CodeIssueGroupingStrategy,
9+
CodeIssueGroupingStrategyState,
10+
SecurityIssueFilters,
11+
SecurityTreeViewFilterState,
12+
} from 'aws-core-vscode/codewhisperer'
813
import { globals } from 'aws-core-vscode/shared'
914

1015
describe('model', function () {
@@ -70,4 +75,100 @@ describe('model', function () {
7075
assert.deepStrictEqual(hiddenSeverities, ['High', 'Low'])
7176
})
7277
})
78+
79+
describe('CodeIssueGroupingStrategyState', function () {
80+
let sandbox: sinon.SinonSandbox
81+
let state: CodeIssueGroupingStrategyState
82+
83+
beforeEach(function () {
84+
sandbox = sinon.createSandbox()
85+
state = CodeIssueGroupingStrategyState.instance
86+
})
87+
88+
afterEach(function () {
89+
sandbox.restore()
90+
})
91+
92+
describe('instance', function () {
93+
it('should return the same instance when called multiple times', function () {
94+
const instance1 = CodeIssueGroupingStrategyState.instance
95+
const instance2 = CodeIssueGroupingStrategyState.instance
96+
assert.strictEqual(instance1, instance2)
97+
})
98+
})
99+
100+
describe('getState', function () {
101+
it('should return fallback when no state is stored', function () {
102+
const result = state.getState()
103+
104+
assert.equal(result, CodeIssueGroupingStrategy.Severity)
105+
})
106+
107+
it('should return stored state when valid', async function () {
108+
const validStrategy = CodeIssueGroupingStrategy.FileLocation
109+
await state.setState(validStrategy)
110+
111+
const result = state.getState()
112+
113+
assert.equal(result, validStrategy)
114+
})
115+
116+
it('should return fallback when stored state is invalid', async function () {
117+
const invalidStrategy = 'invalid'
118+
await state.setState(invalidStrategy)
119+
120+
const result = state.getState()
121+
122+
assert.equal(result, CodeIssueGroupingStrategy.Severity)
123+
})
124+
})
125+
126+
describe('setState', function () {
127+
it('should update state and fire change event for valid strategy', async function () {
128+
const validStrategy = CodeIssueGroupingStrategy.FileLocation
129+
130+
// Create a spy to watch for event emissions
131+
const eventSpy = sandbox.spy()
132+
state.onDidChangeState(eventSpy)
133+
134+
await state.setState(validStrategy)
135+
136+
sinon.assert.calledWith(eventSpy, validStrategy)
137+
})
138+
139+
it('should use fallback and fire change event for invalid strategy', async function () {
140+
const invalidStrategy = 'invalid'
141+
142+
// Create a spy to watch for event emissions
143+
const eventSpy = sandbox.spy()
144+
state.onDidChangeState(eventSpy)
145+
146+
await state.setState(invalidStrategy)
147+
148+
sinon.assert.calledWith(eventSpy, CodeIssueGroupingStrategy.Severity)
149+
})
150+
})
151+
152+
describe('reset', function () {
153+
it('should set state to fallback value', async function () {
154+
const setStateStub = sandbox.stub(state, 'setState').resolves()
155+
156+
await state.reset()
157+
158+
sinon.assert.calledWith(setStateStub, CodeIssueGroupingStrategy.Severity)
159+
})
160+
})
161+
162+
describe('onDidChangeState', function () {
163+
it('should allow subscribing to state changes', async function () {
164+
const listener = sandbox.spy()
165+
const disposable = state.onDidChangeState(listener)
166+
167+
await state.setState(CodeIssueGroupingStrategy.Severity)
168+
169+
sinon.assert.calledWith(listener, CodeIssueGroupingStrategy.Severity)
170+
disposable.dispose()
171+
})
172+
})
173+
})
73174
})

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

Lines changed: 59 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -10,17 +10,24 @@ import {
1010
SecurityTreeViewFilterState,
1111
SecurityIssueProvider,
1212
SeverityItem,
13+
CodeIssueGroupingStrategyState,
14+
CodeIssueGroupingStrategy,
1315
} from 'aws-core-vscode/codewhisperer'
1416
import { createCodeScanIssue } from 'aws-core-vscode/test'
1517
import assert from 'assert'
1618
import sinon from 'sinon'
19+
import path from 'path'
1720

1821
describe('SecurityIssueTreeViewProvider', function () {
19-
let securityIssueProvider: SecurityIssueProvider
2022
let securityIssueTreeViewProvider: SecurityIssueTreeViewProvider
2123

2224
beforeEach(function () {
23-
securityIssueProvider = SecurityIssueProvider.instance
25+
SecurityIssueProvider.instance.issues = [
26+
{ filePath: 'file/path/a', issues: [createCodeScanIssue(), createCodeScanIssue()] },
27+
{ filePath: 'file/path/b', issues: [createCodeScanIssue(), createCodeScanIssue()] },
28+
{ filePath: 'file/path/c', issues: [createCodeScanIssue(), createCodeScanIssue()] },
29+
{ filePath: 'file/path/d', issues: [createCodeScanIssue(), createCodeScanIssue()] },
30+
]
2431
securityIssueTreeViewProvider = new SecurityIssueTreeViewProvider()
2532
})
2633

@@ -44,13 +51,6 @@ describe('SecurityIssueTreeViewProvider', function () {
4451

4552
describe('getChildren', function () {
4653
it('should return sorted list of severities if element is undefined', function () {
47-
securityIssueProvider.issues = [
48-
{ filePath: 'file/path/c', issues: [createCodeScanIssue(), createCodeScanIssue()] },
49-
{ filePath: 'file/path/d', issues: [createCodeScanIssue(), createCodeScanIssue()] },
50-
{ filePath: 'file/path/a', issues: [createCodeScanIssue(), createCodeScanIssue()] },
51-
{ filePath: 'file/path/b', issues: [createCodeScanIssue(), createCodeScanIssue()] },
52-
]
53-
5454
const element = undefined
5555
const result = securityIssueTreeViewProvider.getChildren(element) as SeverityItem[]
5656
assert.strictEqual(result.length, 5)
@@ -102,5 +102,55 @@ describe('SecurityIssueTreeViewProvider', function () {
102102
const result = securityIssueTreeViewProvider.getChildren(element) as IssueItem[]
103103
assert.strictEqual(result.length, 0)
104104
})
105+
106+
it('should return severity-grouped items when grouping strategy is Severity', function () {
107+
sinon.stub(CodeIssueGroupingStrategyState.instance, 'getState').returns(CodeIssueGroupingStrategy.Severity)
108+
109+
const severityItems = securityIssueTreeViewProvider.getChildren() as SeverityItem[]
110+
for (const [index, [severity, expectedIssueCount]] of [
111+
['Critical', 0],
112+
['High', 8],
113+
['Medium', 0],
114+
['Low', 0],
115+
['Info', 0],
116+
].entries()) {
117+
const currentSeverityItem = severityItems[index]
118+
assert.strictEqual(currentSeverityItem.label, severity)
119+
assert.strictEqual(currentSeverityItem.issues.length, expectedIssueCount)
120+
121+
const issueItems = securityIssueTreeViewProvider.getChildren(currentSeverityItem) as IssueItem[]
122+
assert.ok(issueItems.every((item) => item.iconPath === undefined))
123+
assert.ok(
124+
issueItems.every((item) => item.description?.toString().startsWith(path.basename(item.filePath)))
125+
)
126+
}
127+
})
128+
129+
it('should return file-grouped items when grouping strategy is FileLocation', function () {
130+
sinon
131+
.stub(CodeIssueGroupingStrategyState.instance, 'getState')
132+
.returns(CodeIssueGroupingStrategy.FileLocation)
133+
134+
const result = securityIssueTreeViewProvider.getChildren() as FileItem[]
135+
for (const [index, [fileName, expectedIssueCount]] of [
136+
['a', 2],
137+
['b', 2],
138+
['c', 2],
139+
['d', 2],
140+
].entries()) {
141+
const currentFileItem = result[index]
142+
assert.strictEqual(currentFileItem.label, fileName)
143+
assert.strictEqual(currentFileItem.issues.length, expectedIssueCount)
144+
assert.strictEqual(currentFileItem.description, 'file/path')
145+
146+
const issueItems = securityIssueTreeViewProvider.getChildren(currentFileItem) as IssueItem[]
147+
assert.ok(
148+
issueItems.every((item) =>
149+
item.iconPath?.toString().includes(`${item.issue.severity.toLowerCase()}.svg`)
150+
)
151+
)
152+
assert.ok(issueItems.every((item) => item.description?.toString().startsWith('[Ln ')))
153+
}
154+
})
105155
})
106156
})
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
/*!
2+
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
3+
* SPDX-License-Identifier: Apache-2.0
4+
*/
5+
import { createQuickPickPrompterTester, QuickPickPrompterTester } from 'aws-core-vscode/test'
6+
import {
7+
CodeIssueGroupingStrategy,
8+
CodeIssueGroupingStrategyState,
9+
createCodeIssueGroupingStrategyPrompter,
10+
} from 'aws-core-vscode/codewhisperer'
11+
import sinon from 'sinon'
12+
import assert from 'assert'
13+
import vscode from 'vscode'
14+
15+
const severity = { data: CodeIssueGroupingStrategy.Severity, label: 'Severity' }
16+
const fileLocation = { data: CodeIssueGroupingStrategy.FileLocation, label: 'File Location' }
17+
18+
describe('createCodeIssueGroupingStrategyPrompter', function () {
19+
let tester: QuickPickPrompterTester<CodeIssueGroupingStrategy>
20+
21+
beforeEach(function () {
22+
tester = createQuickPickPrompterTester(createCodeIssueGroupingStrategyPrompter())
23+
})
24+
25+
afterEach(function () {
26+
sinon.restore()
27+
})
28+
29+
it('should list grouping strategies', async function () {
30+
tester.assertItems([severity, fileLocation])
31+
tester.hide()
32+
await tester.result()
33+
})
34+
35+
it('should update state on selection', async function () {
36+
const originalState = CodeIssueGroupingStrategyState.instance.getState()
37+
assert.equal(originalState, CodeIssueGroupingStrategy.Severity)
38+
39+
tester.selectItems(fileLocation)
40+
tester.addCallback(() => vscode.commands.executeCommand('workbench.action.acceptSelectedQuickOpenItem'))
41+
42+
await tester.result()
43+
assert.equal(CodeIssueGroupingStrategyState.instance.getState(), fileLocation.data)
44+
})
45+
})

packages/core/package.nls.json

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,7 @@
133133
"AWS.command.amazonq.acceptFix": "Accept Fix",
134134
"AWS.command.amazonq.regenerateFix": "Regenerate Fix",
135135
"AWS.command.amazonq.filterIssues": "Filter Issues",
136+
"AWS.command.amazonq.groupIssues": "Group Issues",
136137
"AWS.command.deploySamApplication": "Deploy SAM Application",
137138
"AWS.command.aboutToolkit": "About",
138139
"AWS.command.downloadLambda": "Download...",
@@ -309,6 +310,10 @@
309310
"AWS.amazonq.scans.projectScanInProgress": "Workspace review is in progress...",
310311
"AWS.amazonq.scans.fileScanInProgress": "File review is in progress...",
311312
"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.",
313+
"AWS.amazonq.scans.severity": "Severity",
314+
"AWS.amazonq.scans.fileLocation": "File Location",
315+
"AWS.amazonq.scans.groupIssues": "Group Issues",
316+
"AWS.amazonq.scans.groupIssues.placeholder": "Select how to group code issues",
312317
"AWS.amazonq.featureDev.error.conversationIdNotFoundError": "Conversation id must exist before starting code generation",
313318
"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>",
314319
"AWS.amazonq.featureDev.error.illegalStateTransition": "Illegal transition between states, restart the conversation",

packages/core/src/amazonqGumby/chat/controller/controller.ts

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -53,11 +53,9 @@ import {
5353
import { CodeTransformTelemetryState } from '../../telemetry/codeTransformTelemetryState'
5454
import DependencyVersions from '../../models/dependencies'
5555
import { getStringHash } from '../../../shared/utilities/textUtilities'
56-
import { getVersionData } from '../../../codewhisperer/service/transformByQ/transformMavenHandler'
5756
import AdmZip from 'adm-zip'
5857
import { AuthError } from '../../../auth/sso/server'
5958
import {
60-
setMaven,
6159
openBuildLogFile,
6260
parseBuildFile,
6361
validateSQLMetadataFile,
@@ -321,12 +319,6 @@ export class GumbyController {
321319
telemetryJavaVersion = JDKToTelemetryValue(javaVersion) as CodeTransformJavaSourceVersionsAllowed
322320
}
323321
telemetry.record({ codeTransformLocalJavaVersion: telemetryJavaVersion })
324-
325-
await setMaven()
326-
const versionInfo = await getVersionData()
327-
const mavenVersionInfoMessage = `${versionInfo[0]} (${transformByQState.getMavenName()})`
328-
telemetry.record({ buildSystemVersion: mavenVersionInfoMessage })
329-
330322
return validProjects
331323
})
332324
return validProjects

packages/core/src/awsService/appBuilder/explorer/detectSamProjects.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ export async function getFiles(
5757

5858
return await vscode.workspace.findFiles(globPattern, excludePattern)
5959
} catch (error) {
60-
getLogger().error(`Failed to get files with pattern ${pattern}:`, error)
60+
getLogger().error(`Failed to find files with pattern ${pattern}:`, error)
6161
return []
6262
}
6363
}

packages/core/src/awsService/appBuilder/explorer/nodes/appNode.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ export class AppNode implements TreeNode {
7373
createPlaceholderItem(
7474
localize(
7575
'AWS.appBuilder.explorerNode.app.noResourceTree',
76-
'[Unable to load Resource tree for this App. Update SAM template]'
76+
'[Unable to load resource tree for this app. Ensure SAM template is correct.]'
7777
)
7878
),
7979
]

0 commit comments

Comments
 (0)