Skip to content

Commit 46643ba

Browse files
authored
chore(amazonq): move getTextDocument to textDocumentUtils and add test (#2382)
1 parent 0980351 commit 46643ba

File tree

6 files changed

+157
-98
lines changed

6 files changed

+157
-98
lines changed

server/aws-lsp-codewhisperer/src/language-server/inline-completion/codeWhispererServer.test.ts

Lines changed: 1 addition & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import { TestFeatures } from '@aws/language-server-runtimes/testing'
1212
import * as assert from 'assert'
1313
import { AWSError } from 'aws-sdk'
1414
import sinon, { StubbedInstance } from 'ts-sinon'
15-
import { CodeWhispererServer, CodewhispererServerFactory, getLanguageIdFromUri } from './codeWhispererServer'
15+
import { CodeWhispererServer, CodewhispererServerFactory } from './codeWhispererServer'
1616
import {
1717
CodeWhispererServiceBase,
1818
CodeWhispererServiceToken,
@@ -2463,54 +2463,6 @@ describe('CodeWhisperer Server', () => {
24632463
})
24642464
})
24652465

2466-
describe('getLanguageIdFromUri', () => {
2467-
it('should return python for notebook cell URIs', () => {
2468-
const uri = 'vscode-notebook-cell:/some/path/notebook.ipynb#cell1'
2469-
assert.strictEqual(getLanguageIdFromUri(uri), 'python')
2470-
})
2471-
2472-
it('should return abap for files with ABAP extensions', () => {
2473-
const uris = ['file:///path/to/file.asprog']
2474-
2475-
uris.forEach(uri => {
2476-
assert.strictEqual(getLanguageIdFromUri(uri), 'abap')
2477-
})
2478-
})
2479-
2480-
it('should return empty string for non-ABAP files', () => {
2481-
const uris = ['file:///path/to/file.js', 'file:///path/to/file.ts', 'file:///path/to/file.py']
2482-
2483-
uris.forEach(uri => {
2484-
assert.strictEqual(getLanguageIdFromUri(uri), '')
2485-
})
2486-
})
2487-
2488-
it('should return empty string for invalid URIs', () => {
2489-
const invalidUris = ['', 'invalid-uri', 'file:///']
2490-
2491-
invalidUris.forEach(uri => {
2492-
assert.strictEqual(getLanguageIdFromUri(uri), '')
2493-
})
2494-
})
2495-
2496-
it('should log errors when provided with a logging object', () => {
2497-
const mockLogger = {
2498-
log: sinon.spy(),
2499-
}
2500-
2501-
const invalidUri = {} as string // Force type error
2502-
getLanguageIdFromUri(invalidUri, mockLogger)
2503-
2504-
sinon.assert.calledOnce(mockLogger.log)
2505-
sinon.assert.calledWith(mockLogger.log, sinon.match(/Error parsing URI to determine language:.*/))
2506-
})
2507-
2508-
it('should handle URIs without extensions', () => {
2509-
const uri = 'file:///path/to/file'
2510-
assert.strictEqual(getLanguageIdFromUri(uri), '')
2511-
})
2512-
})
2513-
25142466
describe('Dynamic Service Manager Selection', () => {
25152467
it('should use Token service manager when not using IAM auth', async () => {
25162468
// Create isolated stubs for this test only

server/aws-lsp-codewhisperer/src/language-server/inline-completion/codeWhispererServer.ts

Lines changed: 1 addition & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,6 @@ import { EditCompletionHandler } from './handler/editCompletionHandler'
2727
import { InlineCompletionHandler } from './handler/inlineCompletionHandler'
2828
import { SessionResultsHandler } from './handler/sessionResultsHandler'
2929
import { isUsingIAMAuth } from '../../shared/utils'
30-
import { TextDocument } from '@aws/language-server-runtimes/server-interface'
31-
import { ABAP_EXTENSIONS } from './contants/constants'
32-
import { URI } from 'vscode-uri'
3330

3431
export const CodewhispererServerFactory =
3532
(serviceManager: (credentialsProvider?: any) => AmazonQBaseServiceManager): Server =>
@@ -179,8 +176,7 @@ export const CodewhispererServerFactory =
179176
credentialsProvider,
180177
() => editsEnabled,
181178
() => timeSinceLastUserModification,
182-
lsp,
183-
getTextDocument
179+
lsp
184180
)
185181

186182
sessionResultsHandler = new SessionResultsHandler(
@@ -302,34 +298,6 @@ export const CodewhispererServerFactory =
302298
}
303299
}
304300

305-
export const getLanguageIdFromUri = (uri: string, logging?: any): string => {
306-
try {
307-
if (uri.startsWith('vscode-notebook-cell:')) {
308-
// use python for now as lsp does not support JL cell language detection
309-
return 'python'
310-
}
311-
const extension = uri.split('.').pop()?.toLowerCase()
312-
return ABAP_EXTENSIONS.has(extension || '') ? 'abap' : ''
313-
} catch (err) {
314-
logging?.log(`Error parsing URI to determine language: ${uri}: ${err}`)
315-
return ''
316-
}
317-
}
318-
319-
export const getTextDocument = async (uri: string, workspace: any, logging: any): Promise<TextDocument | undefined> => {
320-
let textDocument = await workspace.getTextDocument(uri)
321-
if (!textDocument) {
322-
try {
323-
const content = await workspace.fs.readFile(URI.parse(uri).fsPath)
324-
const languageId = getLanguageIdFromUri(uri)
325-
textDocument = TextDocument.create(uri, languageId, 0, content)
326-
} catch (err) {
327-
logging.log(`Unable to load from ${uri}: ${err}`)
328-
}
329-
}
330-
return textDocument
331-
}
332-
333301
// Dynamic service manager factory that detects auth type at runtime
334302
export const CodeWhispererServer = CodewhispererServerFactory((credentialsProvider?: any) => {
335303
return isUsingIAMAuth(credentialsProvider) ? getOrThrowBaseIAMServiceManager() : getOrThrowBaseTokenServiceManager()

server/aws-lsp-codewhisperer/src/language-server/inline-completion/handler/inlineCompletionHandler.test.ts

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import { InlineCompletionTriggerKind, CancellationToken } from '@aws/language-se
1313
import { EMPTY_RESULT } from '../contants/constants'
1414
import * as IdleWorkspaceManagerModule from '../../workspaceContext/IdleWorkspaceManager'
1515
import * as telemetryModule from '../telemetry/telemetry'
16+
import * as textDocumentUtils from '../utils/textDocumentUtils'
1617

1718
describe('InlineCompletionHandler', () => {
1819
const testDocument = TextDocument.create('file:///test.cs', 'csharp', 1, 'test content')
@@ -37,7 +38,7 @@ describe('InlineCompletionHandler', () => {
3738
let credentialsProvider: any
3839
let workspace: any
3940
let logging: any
40-
let getTextDocument: sinon.SinonStub
41+
let getTextDocumentStub: sinon.SinonStub
4142

4243
beforeEach(() => {
4344
SessionManager.reset()
@@ -56,16 +57,16 @@ describe('InlineCompletionHandler', () => {
5657

5758
workspace = { getWorkspaceFolder: sinon.stub() }
5859
logging = { log: sinon.stub(), debug: sinon.stub() }
59-
getTextDocument = sinon.stub().resolves(testDocument)
6060
lsp = { getClientInitializeParams: sinon.stub() } as any
6161
telemetry = { emitMetric: sinon.stub() } as any
6262
credentialsProvider = { getConnectionMetadata: sinon.stub() } as any
6363

64-
// Stub IdleWorkspaceManager and telemetry functions
64+
// Stub IdleWorkspaceManager, telemetry functions, and textDocumentUtils
6565
sinon.stub(IdleWorkspaceManagerModule.IdleWorkspaceManager, 'recordActivityTimestamp')
6666
sinon.stub(telemetryModule, 'emitServiceInvocationTelemetry')
6767
sinon.stub(telemetryModule, 'emitServiceInvocationFailure')
6868
sinon.stub(telemetryModule, 'emitUserTriggerDecisionTelemetry')
69+
getTextDocumentStub = sinon.stub(textDocumentUtils, 'getTextDocument')
6970

7071
handler = new InlineCompletionHandler(
7172
logging,
@@ -82,8 +83,7 @@ describe('InlineCompletionHandler', () => {
8283
credentialsProvider,
8384
() => false,
8485
() => 1000,
85-
lsp,
86-
getTextDocument
86+
lsp
8787
)
8888
})
8989

@@ -117,8 +117,7 @@ describe('InlineCompletionHandler', () => {
117117
{ getConnectionMetadata: sinon.stub() } as any,
118118
() => false,
119119
() => 1000,
120-
{ getClientInitializeParams: sinon.stub() } as any,
121-
getTextDocument
120+
{ getClientInitializeParams: sinon.stub() } as any
122121
)
123122

124123
const result = await handler.onInlineCompletion(completionParams, CancellationToken.None)
@@ -128,7 +127,7 @@ describe('InlineCompletionHandler', () => {
128127
})
129128

130129
it('should return empty result when text document not found', async () => {
131-
getTextDocument.resolves(null)
130+
getTextDocumentStub.resolves(null)
132131

133132
const result = await handler.onInlineCompletion(completionParams, CancellationToken.None)
134133

@@ -137,7 +136,7 @@ describe('InlineCompletionHandler', () => {
137136
})
138137

139138
it('should track cursor position when cursor tracker available', async () => {
140-
getTextDocument.resolves(null) // Will return early
139+
getTextDocumentStub.resolves(null) // Will return early
141140

142141
await handler.onInlineCompletion(completionParams, CancellationToken.None)
143142

server/aws-lsp-codewhisperer/src/language-server/inline-completion/handler/inlineCompletionHandler.ts

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ import {
4848
import { EMPTY_RESULT } from '../contants/constants'
4949
import { IdleWorkspaceManager } from '../../workspaceContext/IdleWorkspaceManager'
5050
import { mergeSuggestionsWithRightContext } from '../utils/mergeRightUtils'
51+
import { getTextDocument } from '../utils/textDocumentUtils'
5152

5253
export class InlineCompletionHandler {
5354
private isOnInlineCompletionHandlerInProgress = false
@@ -67,12 +68,7 @@ export class InlineCompletionHandler {
6768
private readonly credentialsProvider: CredentialsProvider,
6869
private readonly getEditsEnabled: () => boolean,
6970
private readonly getTimeSinceLastUserModification: () => number,
70-
private readonly lsp: Lsp,
71-
private readonly getTextDocument: (
72-
uri: string,
73-
workspace: any,
74-
logging: any
75-
) => Promise<TextDocument | undefined>
71+
private readonly lsp: Lsp
7672
) {}
7773

7874
async onInlineCompletion(
@@ -110,7 +106,7 @@ export class InlineCompletionHandler {
110106
if (this.cursorTracker) {
111107
this.cursorTracker.trackPosition(params.textDocument.uri, params.position)
112108
}
113-
const textDocument = await this.getTextDocument(params.textDocument.uri, this.workspace, this.logging)
109+
const textDocument = await getTextDocument(params.textDocument.uri, this.workspace, this.logging)
114110

115111
const codeWhispererService = this.amazonQServiceManager.getCodewhispererService()
116112
const authType = codeWhispererService instanceof CodeWhispererServiceToken ? 'token' : 'iam'
Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,113 @@
1+
import assert = require('assert')
2+
import { getLanguageIdFromUri, getTextDocument } from './textDocumentUtils'
3+
import { TextDocument } from '@aws/language-server-runtimes/server-interface'
4+
import sinon from 'ts-sinon'
5+
6+
describe('textDocumentUtils', () => {
7+
describe('getLanguageIdFromUri', () => {
8+
it('should return python for notebook cell URIs', () => {
9+
const uri = 'vscode-notebook-cell:/some/path/notebook.ipynb#cell1'
10+
assert.strictEqual(getLanguageIdFromUri(uri), 'python')
11+
})
12+
13+
it('should return abap for files with ABAP extensions', () => {
14+
const uris = ['file:///path/to/file.asprog']
15+
16+
uris.forEach(uri => {
17+
assert.strictEqual(getLanguageIdFromUri(uri), 'abap')
18+
})
19+
})
20+
21+
it('should return empty string for non-ABAP files', () => {
22+
const uris = ['file:///path/to/file.js', 'file:///path/to/file.ts', 'file:///path/to/file.py']
23+
24+
uris.forEach(uri => {
25+
assert.strictEqual(getLanguageIdFromUri(uri), '')
26+
})
27+
})
28+
29+
it('should return empty string for invalid URIs', () => {
30+
const invalidUris = ['', 'invalid-uri', 'file:///']
31+
32+
invalidUris.forEach(uri => {
33+
assert.strictEqual(getLanguageIdFromUri(uri), '')
34+
})
35+
})
36+
37+
it('should log errors when provided with a logging object', () => {
38+
const mockLogger = {
39+
log: sinon.spy(),
40+
}
41+
42+
const invalidUri = {} as string // Force type error
43+
getLanguageIdFromUri(invalidUri, mockLogger)
44+
45+
sinon.assert.calledOnce(mockLogger.log)
46+
sinon.assert.calledWith(mockLogger.log, sinon.match(/Error parsing URI to determine language:.*/))
47+
})
48+
49+
it('should handle URIs without extensions', () => {
50+
const uri = 'file:///path/to/file'
51+
assert.strictEqual(getLanguageIdFromUri(uri), '')
52+
})
53+
})
54+
55+
describe('getTextDocument', () => {
56+
let mockWorkspace: any
57+
let mockLogging: any
58+
59+
beforeEach(() => {
60+
mockWorkspace = {
61+
getTextDocument: sinon.stub(),
62+
fs: {
63+
readFile: sinon.stub(),
64+
},
65+
}
66+
mockLogging = {
67+
log: sinon.stub(),
68+
}
69+
})
70+
71+
it('should return existing text document from workspace', async () => {
72+
const existingDoc = TextDocument.create('file:///test.js', 'javascript', 1, 'content')
73+
mockWorkspace.getTextDocument.resolves(existingDoc)
74+
75+
const result = await getTextDocument('file:///test.js', mockWorkspace, mockLogging)
76+
77+
assert.strictEqual(result, existingDoc)
78+
sinon.assert.calledOnceWithExactly(mockWorkspace.getTextDocument, 'file:///test.js')
79+
sinon.assert.notCalled(mockWorkspace.fs.readFile)
80+
})
81+
82+
it('should create text document from file system when not in workspace', async () => {
83+
mockWorkspace.getTextDocument.resolves(null)
84+
mockWorkspace.fs.readFile.resolves('file content')
85+
86+
const result = await getTextDocument('file:///test.py', mockWorkspace, mockLogging)
87+
88+
assert.strictEqual(result?.uri, 'file:///test.py')
89+
assert.strictEqual(result?.getText(), 'file content')
90+
assert.strictEqual(result?.languageId, '')
91+
sinon.assert.calledOnce(mockWorkspace.fs.readFile)
92+
})
93+
94+
it('should handle file system read errors', async () => {
95+
mockWorkspace.getTextDocument.resolves(null)
96+
mockWorkspace.fs.readFile.rejects(new Error('File not found'))
97+
98+
const result = await getTextDocument('file:///missing.js', mockWorkspace, mockLogging)
99+
100+
assert.strictEqual(result, null)
101+
sinon.assert.calledWith(mockLogging.log, sinon.match(/Unable to load from.*File not found/))
102+
})
103+
104+
it('should use correct language ID for ABAP files', async () => {
105+
mockWorkspace.getTextDocument.resolves(null)
106+
mockWorkspace.fs.readFile.resolves('ABAP content')
107+
108+
const result = await getTextDocument('file:///test.asprog', mockWorkspace, mockLogging)
109+
110+
assert.strictEqual(result?.languageId, 'abap')
111+
})
112+
})
113+
})
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
import { TextDocument } from '@aws/language-server-runtimes/server-interface'
2+
import { ABAP_EXTENSIONS } from '../contants/constants'
3+
import { URI } from 'vscode-uri'
4+
5+
export const getLanguageIdFromUri = (uri: string, logging?: any): string => {
6+
try {
7+
if (uri.startsWith('vscode-notebook-cell:')) {
8+
// use python for now as lsp does not support JL cell language detection
9+
return 'python'
10+
}
11+
const extension = uri.split('.').pop()?.toLowerCase()
12+
return ABAP_EXTENSIONS.has(extension || '') ? 'abap' : ''
13+
} catch (err) {
14+
logging?.log(`Error parsing URI to determine language: ${uri}: ${err}`)
15+
return ''
16+
}
17+
}
18+
19+
export const getTextDocument = async (uri: string, workspace: any, logging: any): Promise<TextDocument | undefined> => {
20+
let textDocument = await workspace.getTextDocument(uri)
21+
if (!textDocument) {
22+
try {
23+
const content = await workspace.fs.readFile(URI.parse(uri).fsPath)
24+
const languageId = getLanguageIdFromUri(uri)
25+
textDocument = TextDocument.create(uri, languageId, 0, content)
26+
} catch (err) {
27+
logging.log(`Unable to load from ${uri}: ${err}`)
28+
}
29+
}
30+
return textDocument
31+
}

0 commit comments

Comments
 (0)