Skip to content
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

Add codeAction support #298

Merged
merged 27 commits into from
Feb 18, 2021
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
90303ff
Addfile-based and plugin code action support
TwitchBronBron Feb 2, 2021
656e2ce
better code action message.
TwitchBronBron Feb 2, 2021
779625e
Merge branch 'master' of https://github.com/rokucommunity/brighterscr…
TwitchBronBron Feb 3, 2021
81daaf4
Add import script tag codeAction
TwitchBronBron Feb 3, 2021
84a53aa
Remove dead code
TwitchBronBron Feb 3, 2021
9d5412a
fix code actions plugin event name
TwitchBronBron Feb 3, 2021
0dfe4ae
Merge branch 'master' into getCodeActions
TwitchBronBron Feb 9, 2021
22b6eca
Remove mutating range
TwitchBronBron Feb 9, 2021
5505867
Fix broken tests.
TwitchBronBron Feb 9, 2021
55e6659
Moved codeActions into internal plugin.
TwitchBronBron Feb 10, 2021
a7a1acc
Fix CI failure issues.
TwitchBronBron Feb 10, 2021
d59f5b7
extends codeAction includes Task and ContentNode
TwitchBronBron Feb 13, 2021
5952cfe
Add extends codeAction to end of component
TwitchBronBron Feb 13, 2021
8b0a122
order codeAction events in interface
TwitchBronBron Feb 13, 2021
3841eae
Merge branch 'master' into getCodeActions
TwitchBronBron Feb 13, 2021
a88fa9d
Merge branch 'master' into getCodeActions
TwitchBronBron Feb 13, 2021
f0c6834
Fix missing logger for PluginInterface
TwitchBronBron Feb 14, 2021
0815c40
Add generic data object for diagnostics.
TwitchBronBron Feb 14, 2021
3a221ad
PluginInterface gets `addFirst` method
TwitchBronBron Feb 14, 2021
ea444c4
Created CodeActionUtil (it doesn't do much yet)
TwitchBronBron Feb 14, 2021
2816575
undo parser references change from separate PR
TwitchBronBron Feb 15, 2021
3cd55a6
Remove xml scope codeAction to simplify PR
TwitchBronBron Feb 15, 2021
2cc7122
Merge branch 'master' into getCodeActions
TwitchBronBron Feb 15, 2021
76f9b04
fix tsc error
TwitchBronBron Feb 15, 2021
f208a46
Remove unused function
TwitchBronBron Feb 15, 2021
a46c522
Merge branch 'master' into getCodeActions
TwitchBronBron Feb 17, 2021
289a445
Merge branch 'master' into getCodeActions
TwitchBronBron Feb 18, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions src/DiagnosticMessages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ export let DiagnosticMessages = {
callToUnknownFunction: (name: string, scopeName: string) => ({
message: `Cannot find function with name '${name}' when this file is included in scope '${scopeName}'`,
code: 1001,
functionName: name,
severity: DiagnosticSeverity.Error
}),
mismatchArgumentCount: (expectedCount: number | string, actualCount: number) => ({
Expand Down Expand Up @@ -608,13 +609,13 @@ export let DiagnosticMessages = {
})
};

let allCodes = [] as number[];
export const DiagnosticCodeMap = {} as Record<keyof (typeof DiagnosticMessages), number>;
export let diagnosticCodes = [] as number[];
for (let key in DiagnosticMessages) {
allCodes.push(DiagnosticMessages[key]().code);
diagnosticCodes.push(DiagnosticMessages[key]().code);
DiagnosticCodeMap[key] = DiagnosticMessages[key]().code;
}

export let diagnosticCodes = allCodes;

export interface DiagnosticInfo {
message: string;
code: number;
Expand Down
27 changes: 25 additions & 2 deletions src/LanguageServer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,17 @@ import type {
DocumentSymbolParams,
ReferenceParams,
SignatureHelp,
SignatureHelpParams
SignatureHelpParams,
CodeActionParams
} from 'vscode-languageserver';
import {
createConnection,
DidChangeConfigurationNotification,
FileChangeType,
ProposedFeatures,
TextDocuments,
TextDocumentSyncKind
TextDocumentSyncKind,
CodeActionKind
} from 'vscode-languageserver';
import { URI } from 'vscode-uri';
import { TextDocument } from 'vscode-languageserver-textdocument';
Expand Down Expand Up @@ -143,6 +145,8 @@ export class LanguageServer {

this.connection.onReferences(this.onReferences.bind(this));

this.connection.onCodeAction(this.onCodeAction.bind(this));

/*
this.connection.onDidOpenTextDocument((params) => {
// A text document got opened in VSCode.
Expand Down Expand Up @@ -196,6 +200,9 @@ export class LanguageServer {
documentSymbolProvider: true,
workspaceSymbolProvider: true,
referencesProvider: true,
codeActionProvider: {
codeActionKinds: [CodeActionKind.Refactor]
},
signatureHelpProvider: {
triggerCharacters: ['(', ',']
},
Expand Down Expand Up @@ -543,6 +550,22 @@ export class LanguageServer {
return item;
}

private async onCodeAction(params: CodeActionParams) {
//ensure programs are initialized
await this.waitAllProgramFirstRuns();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These awaits will potentially accumulate code action requests - should there instead be a mechanism to ignore codeactions when the file isn't ready? We may "miss" some actions but just moving the cursor around will produce then again.

Copy link
Member Author

@TwitchBronBron TwitchBronBron Feb 18, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The language server protocol might already account for this and prevent additional requests if the first one hasn't resolved yet. So I'll test that first, and if we DO get multiple requests at the same time, then I'll add some code to ignore code actions that arrive before the programs are finished with their first runs.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just tested, and I can't get these to queue up. There's a waitAllProgramFirstRuns() in the onInitialized, so the vscode doesn't even send codeAction events until that's resolved, so this line is really a "just in case" measure.


let filePath = util.uriToPath(params.textDocument.uri);

//wait until the file has settled
await this.keyedThrottler.onIdleOnce(filePath, true);

let codeActions = this
.getWorkspaces()
.flatMap(workspace => workspace.builder.program.getCodeActions(filePath, params.range));

return codeActions;
}

/**
* Reload all specified workspaces, or all workspaces if no workspaces are specified
*/
Expand Down
32 changes: 31 additions & 1 deletion src/Program.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import * as assert from 'assert';
import * as fsExtra from 'fs-extra';
import * as path from 'path';
import type { CompletionItem, Position, SignatureInformation } from 'vscode-languageserver';
import type { CodeAction, CompletionItem, Position, Range, SignatureInformation } from 'vscode-languageserver';
import { Location, CompletionItemKind } from 'vscode-languageserver';
import type { BsConfig } from './BsConfig';
import { Scope } from './Scope';
Expand Down Expand Up @@ -733,6 +733,36 @@ export class Program {
return Promise.resolve(file.getHover(position));
}

/**
* Compute code actions for the given file and range
*/
public getCodeActions(pathAbsolute: string, range: Range) {
const codeActions = [] as CodeAction[];
const file = this.getFile(pathAbsolute);

this.plugins.emit('beforeGetCodeActions', file, range, codeActions);

//get code actions from the file
codeActions.push(
...file.getCodeActions(range)
);

//get code actions from every scope this file is a member of
for (let key in this.scopes) {
let scope = this.scopes[key];

if (scope.hasFile(file)) {
//get code actions from each scope
codeActions.push(
...scope.getCodeActions(file, range)
);
}
}

this.plugins.emit('afterGetCodeActions', file, range, codeActions);
return codeActions;
}

public getSignatureHelp(filepath: string, position: Position): SignatureInfoObj[] {
let file: BrsFile = this.getFile(filepath);
if (!file || !isBrsFile(file)) {
Expand Down
7 changes: 6 additions & 1 deletion src/Scope.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import type { CompletionItem, Position, Range } from 'vscode-languageserver';
import type { CodeAction, CompletionItem, Position, Range } from 'vscode-languageserver';
import { CompletionItemKind, Location } from 'vscode-languageserver';
import chalk from 'chalk';
import type { DiagnosticInfo } from './DiagnosticMessages';
Expand Down Expand Up @@ -243,6 +243,11 @@ export class Scope {
this.diagnostics.push(...diagnostics);
}

public getCodeActions(file: BscFile, range: Range) {
const result = [] as CodeAction[];
return result;
}

/**
* Get the list of callables available in this scope (either declared in this scope or in a parent scope)
*/
Expand Down
42 changes: 42 additions & 0 deletions src/XmlScope.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,21 @@ import { Program } from './Program';
import { trim } from './testHelpers.spec';
import { standardizePath as s, util } from './util';
let rootDir = s`${process.cwd()}/rootDir`;
import { createSandbox } from 'sinon';
const sinon = createSandbox();

describe('XmlScope', () => {
let program: Program;
beforeEach(() => {
program = new Program({
rootDir: rootDir
});
sinon.restore();
});

afterEach(() => {
program.dispose();
sinon.restore();
});

describe('constructor', () => {
Expand Down Expand Up @@ -187,4 +191,42 @@ describe('XmlScope', () => {
});
});
});

describe('getCodeActions', () => {
it('sugests import script tag for function from not-imported file', () => {
program.addOrReplaceFile('components/comp1.xml', trim`
<?xml version="1.0" encoding="utf-8" ?>
<component name="child">
<script uri="comp1.brs" />
</component>
`);
const codebehind = program.addOrReplaceFile('components/comp1.brs', `
sub init()
doSomething()
end sub
`);
program.addOrReplaceFile('source/common.brs', `
sub doSomething()
end sub
`);
program.validate();
const stub = sinon.stub(util, 'createCodeAction');
const actions = program.getComponentScope('child').getCodeActions(
codebehind,
// doSo|mething()
util.createRange(2, 24, 2, 24)
);
expect(actions).to.be.lengthOf(1);

expect(stub.getCalls()[0].args[0]).to.eql({
title: `Import "pkg:/source/common.brs" into component "child"`,
changes: [{
filePath: s`${rootDir}/components/comp1.xml`,
newText: ' <script type="text/brightscript" uri="pkg:/source/common.brs" />\n',
type: 'insert',
position: util.createPosition(3, 0)
}]
});
});
});
});
46 changes: 43 additions & 3 deletions src/XmlScope.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
import type { Location, Position } from 'vscode-languageserver';
import type { CodeAction, Location, Position, Range } from 'vscode-languageserver';
import { Scope } from './Scope';
import { DiagnosticMessages } from './DiagnosticMessages';
import { DiagnosticMessages, DiagnosticCodeMap } from './DiagnosticMessages';
import type { XmlFile } from './files/XmlFile';
import type { BscFile, CallableContainerMap, FileReference } from './interfaces';
import type { Program } from './Program';
import util from './util';
import { isXmlFile } from './astUtils/reflection';
import { isBrsFile, isXmlFile } from './astUtils/reflection';
import { SGFieldTypes } from './parser/SGTypes';
import type { SGTag } from './parser/SGTypes';

Expand Down Expand Up @@ -153,6 +153,46 @@ export class XmlScope extends Scope {
});
}

public getCodeActions(file: BscFile, range: Range) {
const result = [] as CodeAction[];
for (const diagnostic of this.diagnostics) {
//skip diagnostics that don't occur on this line
if (diagnostic.range?.start.line !== range.start.line) {
continue;
}
if (diagnostic.code === DiagnosticCodeMap.callToUnknownFunction) {
//functionName is stored on this specific diagnostic
const lowerFunctionName = (diagnostic as any).functionName.toLowerCase();

//find every file with this function defined
for (const key in this.program.files) {
const file = this.program.files[key];
if (isBrsFile(file)) {
//TODO handle namespace-relative function calls
const stmt = file.parser.references.functionStatementLookup.get(lowerFunctionName);
const slashOpenToken = this.xmlFile.parser.ast.component?.ast.SLASH_OPEN?.[0];
if (stmt && slashOpenToken) {
const pkgPath = util.getRokuPkgPath(file.pkgPath);
result.push(
util.createCodeAction({
title: `Import "${pkgPath}" into component "${this.xmlFile.componentName.text ?? this.name}"`,
// diagnostics: [diagnostic]
changes: [{
filePath: this.xmlFile.pathAbsolute,
newText: ` <script type="text/brightscript" uri="${pkgPath}" />\n`,
type: 'insert',
position: util.createPosition(slashOpenToken.startLine - 1, slashOpenToken.startColumn - 1)
}]
})
);
}
}
}
}
}
return result;
}

/**
* Get the definition (where was this thing first defined) of the symbol under the position
*/
Expand Down
6 changes: 5 additions & 1 deletion src/files/BrsFile.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import type { CodeWithSourceMap } from 'source-map';
import { SourceNode } from 'source-map';
import type { CompletionItem, Hover, Range, Position } from 'vscode-languageserver';
import type { CompletionItem, Hover, Range, Position, CodeAction } from 'vscode-languageserver';
import { CompletionItemKind, SymbolKind, Location, SignatureInformation, ParameterInformation, DocumentSymbol, SymbolInformation } from 'vscode-languageserver';
import chalk from 'chalk';
import * as path from 'path';
Expand Down Expand Up @@ -1466,6 +1466,10 @@ export class BrsFile {
}
}

public getCodeActions(range: Range): CodeAction[] {
return [];
}

public getSignatureHelpForNamespaceMethods(callableName: string, dottedGetText: string, scope: Scope): { key: string; signature: SignatureInformation }[] {
if (!dottedGetText) {
return [];
Expand Down
35 changes: 34 additions & 1 deletion src/files/XmlFile.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import * as path from 'path';
import type { CodeWithSourceMap } from 'source-map';
import { SourceNode } from 'source-map';
import type { CompletionItem, Hover, Location, Position, Range } from 'vscode-languageserver';
import type { CodeAction, CompletionItem, Hover, Location, Position, Range } from 'vscode-languageserver';
import { CodeActionKind } from 'vscode-languageserver';
import { DiagnosticMessages } from '../DiagnosticMessages';
import type { FunctionScope } from '../FunctionScope';
import type { Callable, BsDiagnostic, File, FileReference, FunctionCall } from '../interfaces';
Expand Down Expand Up @@ -369,6 +370,38 @@ export class XmlFile {
return null;
}

public getCodeActions(range: Range) {
const result = [] as CodeAction[];

for (const diagnostic of this.diagnostics) {
//skip diagnostics that don't occur on this line
if (diagnostic.range?.start.line !== range.start.line) {
continue;
}
if (diagnostic.code === DiagnosticMessages.xmlComponentMissingExtendsAttribute().code) {
//add the attribute at the end of the first attribute, or after the `<component` if no attributes
const pos = (this.parser.ast.component.attributes[0] ?? this.parser.ast.component.tag).range.end;
//add this extends attribute right after the closing `>` token
range.end.character -= 1;
result.push(
util.createCodeAction({
title: `Add default extends attribute`,
// diagnostics: [diagnostic],
isPreferred: true,
kind: CodeActionKind.QuickFix,
changes: [{
type: 'insert',
filePath: this.pathAbsolute,
position: util.createPosition(pos.line, pos.character),
newText: ' extends="Group"'
}]
})
);
}
}
return result;
}

public getReferences(position: Position): Promise<Location[]> { //eslint-disable-line
//TODO implement
return null;
Expand Down
26 changes: 25 additions & 1 deletion src/interfaces.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import type { Range, Diagnostic } from 'vscode-languageserver';
import type { Range, Diagnostic, CodeAction, Position, CodeActionKind } from 'vscode-languageserver';
import type { Scope } from './Scope';
import type { BrsFile } from './files/BrsFile';
import type { XmlFile } from './files/XmlFile';
Expand Down Expand Up @@ -187,6 +187,8 @@ export interface CompilerPlugin {
afterFileTranspile?: (entry: TranspileObj) => void;
beforeFileDispose?: (file: BscFile) => void;
afterFileDispose?: (file: BscFile) => void;
beforeGetCodeActions?: (file: BscFile, range: Range, codeActions: CodeAction[]) => void;
afterGetCodeActions?: (file: BscFile, range: Range, codeActions: CodeAction[]) => void;
}

export interface TypedefProvider {
Expand All @@ -202,3 +204,25 @@ export interface ExpressionInfo {
varExpressions: Expression[];
uniqueVarNames: string[];
}

export interface CodeActionShorthand {
title: string;
diagnostics?: Diagnostic[];
kind?: CodeActionKind;
isPreferred?: boolean;
changes: Array<InsertChange | ReplaceChange>;
}

export interface InsertChange {
filePath: string;
newText: string;
type: 'insert';
position: Position;
}

export interface ReplaceChange {
filePath: string;
newText: string;
type: 'replace';
range: Range;
}
Loading