Skip to content

Commit 8d7ade2

Browse files
authored
Feature: Automatically restart language client when config or rules files change (#75)
* test: prepare test fixture and activate extension only once, before all other tests * test: added speed target of 1s for tests in general and 3s for initialization test, otherwise mocha display a lot of red text * feat: restart language client if sgconfig.yml or a rule file changes * fix: cleaned up 4 nits. 1) fn name 'restartOnEdits'->'watchEditsToRestartLanguageClient' to convey that it does not immediately restart. 2) unsorted package.json dependencies which were automatically sorted by npm install. 3) removed a commented console.log statement. 4) Undid accidental upgrade of prettier from 3.0.0 to 3.4.0 * fix: attempting to resolve failing windows CI test by waiting longer for initial diagnostics to match their expectations, before actually testing diagnostic changes from file modifications
1 parent 0e41b10 commit 8d7ade2

File tree

11 files changed

+416
-27
lines changed

11 files changed

+416
-27
lines changed

fixture/rules/README.md

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
Make sure that your custom rules in this directory are properly parsed.
2+
`npm run test` will test the LSP which does NOT report errors with rule files (https://github.com/ast-grep/ast-grep/issues/722)
3+
but it will break all of the tests.
4+
5+
You can run
6+
7+
```bash
8+
cd fixture
9+
sg scan
10+
```
11+
12+
to verify that the rules are parsed correctly.

fixture/test.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,3 +9,11 @@ class AnotherCase {
99
return 123
1010
}
1111
}
12+
13+
const NoProblemHere = {
14+
test() {
15+
if (Math.random() > 3) {
16+
throw new Error('This is not an error')
17+
}
18+
}
19+
}

package-lock.json

Lines changed: 16 additions & 6 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

package.json

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,7 @@
7575
"**/*": "prettier --write --ignore-unknown"
7676
},
7777
"dependencies": {
78+
"js-yaml": "^4.1.0",
7879
"vscode-languageclient": "^9.0.0"
7980
},
8081
"devDependencies": {
@@ -96,6 +97,7 @@
9697
"@types/react": "18.2.51",
9798
"@types/react-dom": "18.2.18",
9899
"@types/glob": "^8.0.0",
100+
"@types/js-yaml": "^4.0.9",
99101
"@types/mocha": "^10.0.0",
100102
"@types/node": "^20.0.0",
101103
"@types/vscode": "^1.73.0",

scripts/test/integration-test.cjs

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,16 +7,33 @@ async function run() {
77
// Create the mocha test
88
const mocha = new Mocha({
99
ui: 'tdd',
10-
color: true
10+
color: true,
11+
timeout: 100000,
12+
slow: 1000
1113
})
12-
mocha.timeout(100000)
1314

14-
const testsRoot = path.join(__dirname, '../../src/test')
15+
/**
16+
Before any other tests, run the testSetup.ts file
17+
This will set up the test environment necessary for the other tests to run
18+
*/
19+
mocha.addFile(path.join(__dirname, 'testSetup.ts'))
1520

21+
/**
22+
* Add each test file in src/test/**.test.ts to the test suite
23+
* Tests are run in series and should be designed to not have side effects
24+
* But side effects are possible and therefore test order could matter
25+
* So you could play with sort order on the tests to investigate
26+
*/
1627
try {
28+
const testsRoot = path.join(__dirname, '../../src/test')
1729
let files = await glob('**.test.ts', { cwd: testsRoot })
1830
// Add files to the test suite
19-
files.forEach(f => mocha.addFile(path.resolve(testsRoot, f)))
31+
files.sort((a, b) => {
32+
return a.localeCompare(b)
33+
})
34+
files.forEach(f => {
35+
mocha.addFile(path.resolve(testsRoot, f))
36+
})
2037

2138
return new Promise((resolve, reject) => {
2239
// Run the mocha test

scripts/test/testSetup.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
import { activate } from '../../src/test/utils'
2+
/**
3+
* This initialization runs once, before the rest of the tests.
4+
* It is used to open the test fixture folder and activate the extension.
5+
* We could add some asserts here to make sure the environment looks as expected.
6+
*/
7+
8+
suite('Initializing', () => {
9+
test('Initialize', async () => {
10+
await activate()
11+
}).slow(3000) // This test is slow because it waits for the server to start. And it specifically waits for 2 seconds.
12+
})

src/extension.ts

Lines changed: 96 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,15 +11,21 @@ import {
1111
Position,
1212
TextDocumentShowOptions,
1313
TextDocument,
14+
Disposable,
1415
Uri,
15-
ThemeIcon
16+
ThemeIcon,
17+
FileSystemWatcher,
18+
RelativePattern
1619
} from 'vscode'
1720

21+
import * as yaml from 'js-yaml'
22+
import path from 'path'
1823
import {
1924
LanguageClient,
2025
LanguageClientOptions,
2126
ServerOptions,
22-
Executable
27+
Executable,
28+
State
2329
} from 'vscode-languageclient/node'
2430

2531
import { activate as activateWebview } from './view'
@@ -232,11 +238,99 @@ function activateLsp(context: ExtensionContext) {
232238
client.start()
233239
}
234240

241+
async function watchEditsToRestartLanguageClient(context: ExtensionContext) {
242+
// TODO: handle case where sgconfig.yml is not in root
243+
// TODO: handle case where configPath is not set to sgconfig.yml
244+
// TODO: handle case where no workspace folder is open and then we open one
245+
// TODO: make sure dispose actually does what it should do
246+
// TODO: handle more than 1 element in ruleDirs
247+
248+
if (!workspace.workspaceFolders || !workspace.workspaceFolders[0]) {
249+
return
250+
}
251+
252+
let ruleWatcher: FileSystemWatcher | undefined
253+
let disposables: Disposable[] = []
254+
async function getRuleDirs(configPath: string): Promise<string[]> {
255+
try {
256+
// get the absolute URI to this relative path
257+
let configUri = Uri.file(
258+
workspace.workspaceFolders![0].uri.fsPath + '/' + configPath
259+
)
260+
// read the config file
261+
let configText = await workspace.fs.readFile(configUri)
262+
// parse the config file
263+
let config = yaml.load(configText.toString()) as { ruleDirs: string[] }
264+
// get the rules dir
265+
let ruleDirs = config['ruleDirs']
266+
return ruleDirs
267+
} catch (e) {
268+
console.error('Error parsing config file to find ruleDirs', e)
269+
return ['rules']
270+
}
271+
}
272+
async function updateRuleDirsWatcher() {
273+
/* Assume that we have an open workspace folder, otherwise we won't be able to run anyway */
274+
if (!workspace.workspaceFolders || !workspace.workspaceFolders[0]) {
275+
return
276+
}
277+
let ruleDirsPath = await getRuleDirs('sgconfig.yml')
278+
if (ruleWatcher !== undefined) {
279+
let idx = context.subscriptions.findIndex(item => item === ruleWatcher)
280+
context.subscriptions.splice(idx, 1)
281+
disposables.forEach(item => item.dispose())
282+
disposables = []
283+
}
284+
let rel = path.join(ruleDirsPath[0], '*.yml')
285+
ruleWatcher = workspace.createFileSystemWatcher(
286+
new RelativePattern(workspace.workspaceFolders[0].uri.fsPath, rel)
287+
)
288+
disposables = [
289+
ruleWatcher.onDidChange(uri => {
290+
console.log(`File changed: ${uri.fsPath}`)
291+
restart()
292+
}),
293+
ruleWatcher.onDidCreate(uri => {
294+
console.log(`File created: ${uri.fsPath}`)
295+
restart()
296+
}),
297+
ruleWatcher.onDidDelete(uri => {
298+
console.log(`File deleted: ${uri.fsPath}`)
299+
restart()
300+
})
301+
]
302+
disposables.push(ruleWatcher)
303+
context.subscriptions.push(ruleWatcher)
304+
}
305+
306+
const configFileWatcher = workspace.createFileSystemWatcher(
307+
new RelativePattern(
308+
workspace.workspaceFolders[0].uri.fsPath,
309+
'sgconfig.yml'
310+
)
311+
)
312+
313+
configFileWatcher.onDidChange(uri => {
314+
console.log(`File changed: ${uri.fsPath}`)
315+
updateRuleDirsWatcher()
316+
restart()
317+
})
318+
319+
context.subscriptions.push(configFileWatcher)
320+
updateRuleDirsWatcher()
321+
}
322+
235323
export function activate(context: ExtensionContext) {
324+
watchEditsToRestartLanguageClient(context)
236325
activateLsp(context)
237326
activateWebview(context)
238327
}
239328

329+
async function restart() {
330+
await deactivate()
331+
return await client.start()
332+
}
333+
240334
workspace.onDidChangeConfiguration(changeEvent => {
241335
if (changeEvent.affectsConfiguration('astGrep')) {
242336
deactivate()

src/test/codefix.test.ts

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import * as vscode from 'vscode'
22
import * as assert from 'assert'
33

4-
import { getDocUri, activate } from './utils'
4+
import { getDocUri } from './utils'
55

66
suite('Should get code action', () => {
77
const docUri = getDocUri('test.ts')
@@ -37,8 +37,6 @@ async function testCodeFix(
3737
range: vscode.Range,
3838
expectedCodeActions: vscode.CodeAction[]
3939
) {
40-
await activate(docUri)
41-
4240
const actualCodeActions = (await vscode.commands.executeCommand(
4341
'vscode.executeCodeActionProvider',
4442
docUri,

src/test/diagnostics.test.ts

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import * as vscode from 'vscode'
22
import * as assert from 'assert'
33

4-
import { getDocUri, activate } from './utils'
4+
import { getDocUri } from './utils'
55

66
suite('Should get diagnostics', () => {
77
const docUri = getDocUri('test.ts')
@@ -39,8 +39,6 @@ async function testDiagnostics(
3939
docUri: vscode.Uri,
4040
expectedDiagnostics: vscode.Diagnostic[]
4141
) {
42-
await activate(docUri)
43-
4442
const actualDiagnostics = vscode.languages.getDiagnostics(docUri)
4543
actualDiagnostics.sort((a, b) =>
4644
a.message === b.message ? 0 : a.message > b.message ? 1 : -1

0 commit comments

Comments
 (0)