-
Notifications
You must be signed in to change notification settings - Fork 747
telemetry(amazonq): Add changed IDE diagnostics after user acceptance #7130
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from 17 commits
Commits
Show all changes
18 commits
Select commit
Hold shift + click to select a range
c85deae
add diagnostics
leigaol 37268cf
chat telemetry
leigaol 6df6f26
diagnostics
leigaol acc1663
diagnostic severity
leigaol c7f16d0
simplify
leigaol 91a4771
only fetch diagnostics on accept
leigaol bafad7f
dedup added/removed
leigaol 12d9e48
simplify
leigaol 0ecbbb5
merge
leigaol ab9e996
add diagnostics
leigaol b93592e
simplify
leigaol 880ab84
fix unit test
leigaol b81dd43
update scope
leigaol 4d13116
fix test case
leigaol 9d55648
fix linter
leigaol 99f6575
fix range
leigaol 46354e8
Merge branch 'master' of github.com:leigaol/aws-toolkit-vscode into d…
leigaol 705c84f
Merge branch 'master' into diagnostics
leigaol File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
92 changes: 92 additions & 0 deletions
92
packages/amazonq/test/unit/codewhisperer/util/diagnosticsUtil.test.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,92 @@ | ||
| /*! | ||
| * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
|
|
||
| import * as assert from 'assert' | ||
| import * as vscode from 'vscode' | ||
| import { getDiagnosticsType, getDiagnosticsDifferences } from 'aws-core-vscode/codewhisperer' | ||
| describe('diagnosticsUtil', function () { | ||
| describe('getDiagnosticsType', function () { | ||
| it('should identify SYNTAX_ERROR correctly', function () { | ||
| assert.strictEqual(getDiagnosticsType('Expected semicolon'), 'SYNTAX_ERROR') | ||
| assert.strictEqual(getDiagnosticsType('Incorrect indent level'), 'SYNTAX_ERROR') | ||
| assert.strictEqual(getDiagnosticsType('Syntax error in line 5'), 'SYNTAX_ERROR') | ||
| }) | ||
|
|
||
| it('should identify TYPE_ERROR correctly', function () { | ||
| assert.strictEqual(getDiagnosticsType('Type mismatch'), 'TYPE_ERROR') | ||
| assert.strictEqual(getDiagnosticsType('Invalid type cast'), 'TYPE_ERROR') | ||
| }) | ||
|
|
||
| it('should identify REFERENCE_ERROR correctly', function () { | ||
| assert.strictEqual(getDiagnosticsType('Variable is undefined'), 'REFERENCE_ERROR') | ||
| assert.strictEqual(getDiagnosticsType('Variable not defined'), 'REFERENCE_ERROR') | ||
| assert.strictEqual(getDiagnosticsType('Reference error occurred'), 'REFERENCE_ERROR') | ||
| }) | ||
|
|
||
| it('should identify BEST_PRACTICE correctly', function () { | ||
| assert.strictEqual(getDiagnosticsType('Using deprecated method'), 'BEST_PRACTICE') | ||
| assert.strictEqual(getDiagnosticsType('Variable is unused'), 'BEST_PRACTICE') | ||
| assert.strictEqual(getDiagnosticsType('Variable not initialized'), 'BEST_PRACTICE') | ||
| }) | ||
|
|
||
| it('should identify SECURITY correctly', function () { | ||
| assert.strictEqual(getDiagnosticsType('Potential security vulnerability'), 'SECURITY') | ||
| assert.strictEqual(getDiagnosticsType('Security risk detected'), 'SECURITY') | ||
| }) | ||
|
|
||
| it('should return OTHER for unrecognized messages', function () { | ||
| assert.strictEqual(getDiagnosticsType('Random message'), 'OTHER') | ||
| assert.strictEqual(getDiagnosticsType(''), 'OTHER') | ||
| }) | ||
| }) | ||
|
|
||
| describe('getDiagnosticsDifferences', function () { | ||
| const createDiagnostic = (message: string): vscode.Diagnostic => { | ||
| return { | ||
| message, | ||
| severity: vscode.DiagnosticSeverity.Error, | ||
| range: new vscode.Range(0, 0, 0, 1), | ||
| source: 'test', | ||
| } | ||
| } | ||
|
|
||
| it('should return empty arrays when both inputs are undefined', function () { | ||
| const result = getDiagnosticsDifferences(undefined, undefined) | ||
| assert.deepStrictEqual(result, { added: [], removed: [] }) | ||
| }) | ||
|
|
||
| it('should return empty arrays when filepaths are different', function () { | ||
| const oldDiagnostics = { | ||
| filepath: '/path/to/file1', | ||
| diagnostics: [createDiagnostic('error1')], | ||
| } | ||
| const newDiagnostics = { | ||
| filepath: '/path/to/file2', | ||
| diagnostics: [createDiagnostic('error1')], | ||
| } | ||
| const result = getDiagnosticsDifferences(oldDiagnostics, newDiagnostics) | ||
| assert.deepStrictEqual(result, { added: [], removed: [] }) | ||
| }) | ||
|
|
||
| it('should correctly identify added and removed diagnostics', function () { | ||
| const diagnostic1 = createDiagnostic('error1') | ||
| const diagnostic2 = createDiagnostic('error2') | ||
| const diagnostic3 = createDiagnostic('error3') | ||
|
|
||
| const oldDiagnostics = { | ||
| filepath: '/path/to/file', | ||
| diagnostics: [diagnostic1, diagnostic2], | ||
| } | ||
| const newDiagnostics = { | ||
| filepath: '/path/to/file', | ||
| diagnostics: [diagnostic2, diagnostic3], | ||
| } | ||
|
|
||
| const result = getDiagnosticsDifferences(oldDiagnostics, newDiagnostics) | ||
| assert.deepStrictEqual(result.added, [diagnostic3]) | ||
| assert.deepStrictEqual(result.removed, [diagnostic1]) | ||
| }) | ||
| }) | ||
| }) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
117 changes: 117 additions & 0 deletions
117
packages/core/src/codewhisperer/util/diagnosticsUtil.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,117 @@ | ||
| /*! | ||
| * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
| import * as vscode from 'vscode' | ||
| import * as crypto from 'crypto' | ||
| import { IdeDiagnostic } from '../client/codewhispereruserclient' | ||
|
|
||
| export function getDiagnosticsOfCurrentFile(): FileDiagnostic | undefined { | ||
| if (vscode.window.activeTextEditor) { | ||
| return { | ||
| diagnostics: vscode.languages.getDiagnostics(vscode.window.activeTextEditor.document.uri), | ||
| filepath: vscode.window.activeTextEditor.document.uri.fsPath, | ||
| } | ||
| } | ||
| return undefined | ||
| } | ||
|
|
||
| export type FileDiagnostic = { | ||
| filepath: string | ||
| diagnostics: vscode.Diagnostic[] | ||
| } | ||
|
|
||
| export function getDiagnosticsDifferences( | ||
| oldDiagnostics: FileDiagnostic | undefined, | ||
| newDiagnostics: FileDiagnostic | undefined | ||
| ): { added: vscode.Diagnostic[]; removed: vscode.Diagnostic[] } { | ||
| const result: { added: vscode.Diagnostic[]; removed: vscode.Diagnostic[] } = { added: [], removed: [] } | ||
| if ( | ||
| oldDiagnostics === undefined || | ||
| newDiagnostics === undefined || | ||
| newDiagnostics.filepath !== oldDiagnostics.filepath | ||
| ) { | ||
| return result | ||
| } | ||
|
|
||
| // Create maps using diagnostic key for uniqueness | ||
| const oldMap = new Map(oldDiagnostics.diagnostics.map((d) => [getDiagnosticKey(d), d])) | ||
| const newMap = new Map(newDiagnostics.diagnostics.map((d) => [getDiagnosticKey(d), d])) | ||
|
|
||
| // Get added diagnostics (in new but not in old) | ||
| result.added = [...newMap.values()].filter((d) => !oldMap.has(getDiagnosticKey(d))) | ||
|
|
||
| // Get removed diagnostics (in old but not in new) | ||
| result.removed = [...oldMap.values()].filter((d) => !newMap.has(getDiagnosticKey(d))) | ||
|
|
||
| return result | ||
| } | ||
|
|
||
| export function toIdeDiagnostics(diagnostic: vscode.Diagnostic): IdeDiagnostic { | ||
| const severity = | ||
| diagnostic.severity === vscode.DiagnosticSeverity.Error | ||
| ? 'ERROR' | ||
| : diagnostic.severity === vscode.DiagnosticSeverity.Warning | ||
| ? 'WARNING' | ||
| : diagnostic.severity === vscode.DiagnosticSeverity.Hint | ||
| ? 'HINT' | ||
| : 'INFORMATION' | ||
|
|
||
| return { | ||
| ideDiagnosticType: getDiagnosticsType(diagnostic.message), | ||
| severity: severity, | ||
| source: diagnostic.source, | ||
| range: { | ||
| start: { | ||
| line: diagnostic.range.start.line, | ||
| character: diagnostic.range.start.character, | ||
| }, | ||
| end: { | ||
| line: diagnostic.range.end.line, | ||
| character: diagnostic.range.end.character, | ||
| }, | ||
| }, | ||
| } | ||
| } | ||
|
|
||
| export function getDiagnosticsType(message: string): string { | ||
| const errorTypes = new Map([ | ||
| ['SYNTAX_ERROR', ['expected', 'indent', 'syntax']], | ||
| ['TYPE_ERROR', ['type', 'cast']], | ||
| ['REFERENCE_ERROR', ['undefined', 'not defined', 'undeclared', 'reference']], | ||
| ['BEST_PRACTICE', ['deprecated', 'unused', 'uninitialized', 'not initialized']], | ||
| ['SECURITY', ['security', 'vulnerability']], | ||
| ]) | ||
|
|
||
| const lowercaseMessage = message.toLowerCase() | ||
|
|
||
| for (const [errorType, keywords] of errorTypes) { | ||
| if (keywords.some((keyword) => lowercaseMessage.includes(keyword))) { | ||
| return errorType | ||
| } | ||
| } | ||
|
|
||
| return 'OTHER' | ||
| } | ||
|
|
||
| /** | ||
| * Generates a unique MD5 hash key for a VS Code diagnostic object. | ||
| * | ||
| * @param diagnostic - A VS Code Diagnostic object containing information about a code diagnostic | ||
| * @returns A 32-character hexadecimal MD5 hash string that uniquely identifies the diagnostic | ||
| * | ||
| * @description | ||
| * Creates a deterministic hash by combining the diagnostic's message, severity, code, and source. | ||
| * This hash can be used as a unique identifier for deduplication or tracking purposes. | ||
| * Note: range is not in the hashed string because a diagnostic can move and its range can change within the editor | ||
| */ | ||
| function getDiagnosticKey(diagnostic: vscode.Diagnostic): string { | ||
| const jsonStr = JSON.stringify({ | ||
| message: diagnostic.message, | ||
| severity: diagnostic.severity, | ||
| code: diagnostic.code, | ||
| source: diagnostic.source, | ||
| }) | ||
|
|
||
| return crypto.createHash('md5').update(jsonStr).digest('hex') | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Curious if the sleep here could pollute the data. For users who accept and continue to edit, resulting in diagnostics of WIP code.
Service side has the final say, but an alternate could be aborting with onDidChangeTextDocument event.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a requirement from server side. They know this metrics can be noisy because we rely on the 3rd party plugins for reporting these problems.