Merge pull request #5400 from 50Wliu/users/winstonliu/feature-nullability

Feature nullability

Author: JoeRobich

package.json

@@ -1189,17 +1189,20 @@
       {
         "command": "o.fixAll.solution",
         "title": "Fix all occurrences of a code issue within solution",
-        "category": "OmniSharp"
+        "category": "OmniSharp",
+        "enablement": "editorFocus"
       },
       {
         "command": "o.fixAll.project",
         "title": "Fix all occurrences of a code issue within project",
-        "category": "OmniSharp"
+        "category": "OmniSharp",
+        "enablement": "editorFocus"
       },
       {
         "command": "o.fixAll.document",
         "title": "Fix all occurrences of a code issue within document",
-        "category": "OmniSharp"
+        "category": "OmniSharp",
+        "enablement": "editorFocus"
       },
       {
         "command": "o.reanalyze.allProjects",
@@ -1209,7 +1212,8 @@
       {
         "command": "o.reanalyze.currentProject",
         "title": "Analyze current project",
-        "category": "OmniSharp"
+        "category": "OmniSharp",
+        "enablement": "editorFocus"
       },
       {
         "command": "dotnet.generateAssets",

src/features/codeActionProvider.ts

@@ -23,7 +23,7 @@ export default class CodeActionProvider extends AbstractProvider implements vsco
         this.addDisposables(new CompositeDisposable(registerCommandDisposable));
     }
 
-    public async provideCodeActions(document: vscode.TextDocument, range: vscode.Range | vscode.Selection, context: vscode.CodeActionContext, token: vscode.CancellationToken): Promise<vscode.CodeAction[]> {
+    public async provideCodeActions(document: vscode.TextDocument, range: vscode.Range | vscode.Selection, context: vscode.CodeActionContext, token: vscode.CancellationToken): Promise<vscode.CodeAction[] | undefined> {
         const options = this.optionProvider.GetLatestOptions();
         if (options.disableCodeActions) {
             return;
@@ -75,7 +75,7 @@ export default class CodeActionProvider extends AbstractProvider implements vsco
         }
     }
 
-    private mapOmniSharpCodeActionKindToVSCodeCodeActionKind(kind: string): vscode.CodeActionKind {
+    private mapOmniSharpCodeActionKindToVSCodeCodeActionKind(kind: string | undefined): vscode.CodeActionKind {
         switch (kind) {
             case 'QuickFix':
                 return vscode.CodeActionKind.QuickFix;
@@ -90,7 +90,7 @@ export default class CodeActionProvider extends AbstractProvider implements vsco
         }
     }
 
-    private async _runCodeAction(req: protocol.V2.RunCodeActionRequest, token: vscode.CancellationToken): Promise<boolean | string | {}> {
+    private async _runCodeAction(req: protocol.V2.RunCodeActionRequest, token: vscode.CancellationToken): Promise<boolean | string | undefined> {
         try {
             const response = await serverUtils.runCodeAction(this._server, req);
             if (response) {
@@ -99,5 +99,7 @@ export default class CodeActionProvider extends AbstractProvider implements vsco
         } catch (error) {
             return Promise.reject(`Problem invoking 'RunCodeAction' on OmniSharp server: ${error}`);
         }
+
+        return undefined;
     }
 }

src/features/codeLensProvider.ts

@@ -77,6 +77,11 @@ class DebugTestsCodeLens extends TestCodeLens {
     }
 }
 
+interface Test {
+    framework: string;
+    methodName: string;
+}
+
 export default class OmniSharpCodeLensProvider extends AbstractProvider implements vscode.CodeLensProvider {
 
     constructor(server: OmniSharpServer, testManager: TestManager, private optionProvider: OptionProvider, languageMiddlewareFeature: LanguageMiddlewareFeature) {
@@ -100,7 +105,7 @@ export default class OmniSharpCodeLensProvider extends AbstractProvider implemen
         return [];
     }
 
-    async resolveCodeLens(codeLens: vscode.CodeLens, token: vscode.CancellationToken): Promise<vscode.CodeLens> {
+    async resolveCodeLens(codeLens: vscode.CodeLens, token: vscode.CancellationToken): Promise<vscode.CodeLens | undefined> {
         if (codeLens instanceof ReferencesCodeLens) {
             return this.resolveReferencesCodeLens(codeLens, token);
         }
@@ -110,9 +115,11 @@ export default class OmniSharpCodeLensProvider extends AbstractProvider implemen
         else if (codeLens instanceof DebugTestsCodeLens) {
             return this.resolveTestCodeLens(codeLens, 'Debug Test', 'dotnet.test.debug', 'Debug All Tests', 'dotnet.classTests.debug');
         }
+
+        return undefined;
     }
 
-    private async resolveReferencesCodeLens(codeLens: ReferencesCodeLens, token: vscode.CancellationToken): Promise<vscode.CodeLens> {
+    private async resolveReferencesCodeLens(codeLens: ReferencesCodeLens, token: vscode.CancellationToken): Promise<vscode.CodeLens | undefined> {
         const request: protocol.FindUsagesRequest = {
             FileName: codeLens.fileName,
             Line: codeLens.range.start.line,
@@ -137,7 +144,7 @@ export default class OmniSharpCodeLensProvider extends AbstractProvider implemen
             codeLens.command = {
                 title: count === 1 ? '1 reference' : `${count} references`,
                 command: 'editor.action.showReferences',
-                arguments: [vscode.Uri.file(request.FileName), codeLens.range.start, remappedLocations]
+                arguments: [vscode.Uri.file(codeLens.fileName), codeLens.range.start, remappedLocations]
             };
 
             return codeLens;
@@ -147,7 +154,7 @@ export default class OmniSharpCodeLensProvider extends AbstractProvider implemen
         }
     }
 
-    private async resolveTestCodeLens(codeLens: TestCodeLens, singularTitle: string, singularCommandName: string, pluralTitle: string, pluralCommandName: string): Promise<vscode.CodeLens> {
+    private async resolveTestCodeLens(codeLens: TestCodeLens, singularTitle: string, singularCommandName: string, pluralTitle: string, pluralCommandName: string): Promise<vscode.CodeLens | undefined> {
         if (!codeLens.isTestContainer) {
             // This is just a single test method, not a container.
             codeLens.command = {
@@ -202,35 +209,33 @@ function createCodeLensesForElement(element: Structure.CodeElement, fileName: st
     }
 
     if (options.showTestsCodeLens) {
-        if (isValidMethodForTestCodeLens(element)) {
-            let [testFramework, testMethodName] = getTestFrameworkAndMethodName(element);
-            let range = element.Ranges[SymbolRangeNames.Name];
-
-            if (range && testFramework && testMethodName) {
-                results.push(new RunTestsCodeLens(range, fileName, element.DisplayName,/*isTestContainer*/ false, testFramework, [testMethodName]));
-                results.push(new DebugTestsCodeLens(range, fileName, element.DisplayName,/*isTestContainer*/ false, testFramework, [testMethodName]));
+        if (element.Kind === SymbolKinds.Method) {
+            const test = getTest(element);
+            if (test !== undefined) {
+                let range = element.Ranges[SymbolRangeNames.Name];
+                if (range !== undefined) {
+                    results.push(new RunTestsCodeLens(range, fileName, element.DisplayName,/*isTestContainer*/ false, test.framework, [test.methodName]));
+                    results.push(new DebugTestsCodeLens(range, fileName, element.DisplayName,/*isTestContainer*/ false, test.framework, [test.methodName]));
+                }
             }
-        }
-        else if (isValidClassForTestCodeLens(element)) {
-            // Note: We don't handle multiple test frameworks in the same class. The first test framework wins.
-            let testFramework: string = null;
-            let testMethodNames: string[] = [];
-            let range = element.Ranges[SymbolRangeNames.Name];
+        } else if (element.Kind === SymbolKinds.Class && element.Children !== undefined) {
+            const methods = element.Children.filter(child => child.Kind === SymbolKinds.Method);
 
-            for (let childElement of element.Children) {
-                let [childTestFramework, childTestMethodName] = getTestFrameworkAndMethodName(childElement);
+            const tests = methods
+                .map(method => getTest(method))
+                .filter((test): test is NonNullable<typeof test> => test !== undefined);
 
-                if (!testFramework && childTestFramework) {
-                    testFramework = childTestFramework;
-                    testMethodNames.push(childTestMethodName);
-                }
-                else if (testFramework && childTestFramework === testFramework) {
-                    testMethodNames.push(childTestMethodName);
-                }
+            // Note: We don't handle multiple test frameworks in the same class. The first test framework wins.
+            const testFramework = tests.length > 0 ? tests[0].framework : undefined;
+            if (testFramework !== undefined) {
+                const testMethodNames = tests
+                    .filter(test => test.framework === testFramework)
+                    .map(test => test.methodName);
+
+                let range = element.Ranges[SymbolRangeNames.Name];
+                results.push(new RunTestsCodeLens(range, fileName, element.DisplayName,/*isTestContainer*/ true, testFramework, testMethodNames));
+                results.push(new DebugTestsCodeLens(range, fileName, element.DisplayName,/*isTestContainer*/ true, testFramework, testMethodNames));
             }
-
-            results.push(new RunTestsCodeLens(range, fileName, element.DisplayName,/*isTestContainer*/ true, testFramework, testMethodNames));
-            results.push(new DebugTestsCodeLens(range, fileName, element.DisplayName,/*isTestContainer*/ true, testFramework, testMethodNames));
         }
     }
 
@@ -262,40 +267,19 @@ function isValidElementForReferencesCodeLens(element: Structure.CodeElement, opt
     return true;
 }
 
-
-function isValidClassForTestCodeLens(element: Structure.CodeElement): boolean {
-    if (element.Kind != SymbolKinds.Class) {
-        return false;
+function getTest(element: Structure.CodeElement): Test | undefined {
+    if (element.Properties === undefined) {
+        return undefined;
     }
 
-    if (!element.Children) {
-        return false;
+    const framework = element.Properties[SymbolPropertyNames.TestFramework];
+    const methodName = element.Properties[SymbolPropertyNames.TestMethodName];
+    if (framework === undefined || methodName === undefined) {
+        return undefined;
     }
 
-    return element.Children.find(isValidMethodForTestCodeLens) !== undefined;
-}
-
-function isValidMethodForTestCodeLens(element: Structure.CodeElement): boolean {
-    if (element.Kind != SymbolKinds.Method) {
-        return false;
+    return {
+        framework,
+        methodName,
     }
-
-    if (!element.Properties ||
-        !element.Properties[SymbolPropertyNames.TestFramework] ||
-        !element.Properties[SymbolPropertyNames.TestMethodName]) {
-        return false;
-    }
-
-    return true;
-}
-
-function getTestFrameworkAndMethodName(element: Structure.CodeElement): [string, string] {
-    if (!element.Properties) {
-        return [null, null];
-    }
-
-    const testFramework = element.Properties[SymbolPropertyNames.TestFramework];
-    const testMethodName = element.Properties[SymbolPropertyNames.TestMethodName];
-
-    return [testFramework, testMethodName];
 }

src/features/commands.ts

@@ -53,7 +53,7 @@ export default function registerCommands(context: vscode.ExtensionContext, serve
     // Register command for generating tasks.json and launch.json assets.
     disposable.add(vscode.commands.registerCommand('dotnet.generateAssets', async (selectedIndex) => generateAssets(server, selectedIndex)));
 
-    disposable.add(vscode.commands.registerCommand('csharp.reportIssue', async () => reportIssue(vscode, eventStream, getDotnetInfo, platformInfo.isValidPlatformForMono(), optionProvider.GetLatestOptions(), monoResolver, dotnetResolver)));
+    disposable.add(vscode.commands.registerCommand('csharp.reportIssue', async () => reportIssue(vscode, context.extension.packageJSON.version, eventStream, getDotnetInfo, platformInfo.isValidPlatformForMono(), optionProvider.GetLatestOptions(), dotnetResolver, monoResolver)));
 
     disposable.add(vscode.commands.registerCommand('csharp.showDecompilationTerms', async () => showDecompilationTerms(context, server, optionProvider)));
 
@@ -154,8 +154,13 @@ async function reAnalyzeAllProjects(server: OmniSharpServer, eventStream: EventS
 }
 
 async function reAnalyzeCurrentProject(server: OmniSharpServer, eventStream: EventStream): Promise<void> {
+    const activeTextEditor = vscode.window.activeTextEditor;
+    if (activeTextEditor === undefined) {
+        return;
+    }
+
     await serverUtils.reAnalyze(server, {
-        fileName: vscode.window.activeTextEditor.document.uri.fsPath
+        FileName: activeTextEditor.document.uri.fsPath,
     });
 }
 

src/features/completionProvider.ts

@@ -23,14 +23,14 @@ export default class OmnisharpCompletionProvider extends AbstractProvider implem
         super(server, languageMiddlewareFeature);
     }
 
-    public async provideCompletionItems(document: TextDocument, position: Position, token: CancellationToken, context: CompletionContext): Promise<CompletionList> {
+    public async provideCompletionItems(document: TextDocument, position: Position, token: CancellationToken, context: CompletionContext): Promise<CompletionList | undefined> {
         let request = createRequest<protocol.CompletionRequest>(document, position);
         request.CompletionTrigger = (context.triggerKind + 1) as LspCompletionTriggerKind;
         request.TriggerCharacter = context.triggerCharacter;
 
         try {
             const response = await serverUtils.getCompletion(this._server, request, token);
-            let mappedItems = response.Items.map(arg => this._convertToVscodeCompletionItem(arg));
+            let mappedItems = response.Items.map(arg => this._convertToVscodeCompletionItem(arg, document));
 
             if (isVirtualCSharpDocument(document)) {
                 // The `await` completion item is not compatible with all Razor scenarios.
@@ -89,16 +89,16 @@ export default class OmnisharpCompletionProvider extends AbstractProvider implem
         const request: protocol.CompletionResolveRequest = { Item: lspItem };
         try {
             const response = await serverUtils.getCompletionResolve(this._server, request, token);
-            return this._convertToVscodeCompletionItem(response.Item);
+            return this._convertToVscodeCompletionItem(response.Item, item.command?.arguments?.[1] as TextDocument);
         }
         catch (error) {
-            return;
+            return item;
         }
     }
 
-    public async afterInsert(item: protocol.OmnisharpCompletionItem) {
+    public async afterInsert(item: protocol.OmnisharpCompletionItem, document: TextDocument) {
         try {
-            const uri = window.activeTextEditor.document.uri;
+            const uri = document.uri;
             const response = await serverUtils.getCompletionAfterInsert(this._server, { Item: item });
 
             if (!response.Changes || !response.Column || !response.Line) {
@@ -119,18 +119,20 @@ export default class OmnisharpCompletionProvider extends AbstractProvider implem
                 return;
             }
 
-            const responseLine = response.Line;
-            const responseColumn = response.Column;
+            const editor = window.visibleTextEditors.find(editor => editor.document === document);
+            if (editor === undefined) {
+                return;
+            }
 
-            const finalPosition = new Position(responseLine, responseColumn);
-            window.activeTextEditor.selections = [new Selection(finalPosition, finalPosition)];
+            const finalPosition = new Position(response.Line, response.Column);
+            editor.selections = [new Selection(finalPosition, finalPosition)];
         }
         catch (error) {
             return;
         }
     }
 
-    private _convertToVscodeCompletionItem(omnisharpCompletion: protocol.OmnisharpCompletionItem): CompletionItem {
+    private _convertToVscodeCompletionItem(omnisharpCompletion: protocol.OmnisharpCompletionItem, document: TextDocument): CompletionItem {
         const docs: MarkdownString | undefined = omnisharpCompletion.Documentation ? new MarkdownString(omnisharpCompletion.Documentation, false) : undefined;
 
         const mapRange = function (edit: protocol.LinePositionSpanTextChange): Range {
@@ -165,7 +167,7 @@ export default class OmnisharpCompletionProvider extends AbstractProvider implem
             sortText: omnisharpCompletion.SortText,
             additionalTextEdits: additionalTextEdits,
             keepWhitespace: true,
-            command: omnisharpCompletion.HasAfterInsertStep ? { command: CompletionAfterInsertCommand, title: "", arguments: [omnisharpCompletion] } : undefined
+            command: omnisharpCompletion.HasAfterInsertStep ? { command: CompletionAfterInsertCommand, title: "", arguments: [omnisharpCompletion, document] } : undefined
         };
     }
 }

src/features/definitionMetadataDocumentProvider.ts

@@ -9,7 +9,7 @@ import { IDisposable } from '../Disposable';
 
 export default class DefinitionMetadataDocumentProvider implements TextDocumentContentProvider, IDisposable {
     readonly scheme = "omnisharp-metadata";
-    private _registration: IDisposable;
+    private _registration?: IDisposable;
     private _documents: Map<string, MetadataResponse>;
     private _documentClosedSubscription: IDisposable;
 
@@ -23,7 +23,7 @@ export default class DefinitionMetadataDocumentProvider implements TextDocumentC
     }
 
     public dispose(): void {
-        this._registration.dispose();
+        this._registration?.dispose();
         this._documentClosedSubscription.dispose();
         this._documents.clear();
     }
@@ -48,12 +48,12 @@ export default class DefinitionMetadataDocumentProvider implements TextDocumentC
         this._registration = workspace.registerTextDocumentContentProvider(this.scheme, this);
     }
 
-    public provideTextDocumentContent(uri: Uri): string {
+    public provideTextDocumentContent(uri: Uri): string | undefined {
         return this._documents.get(uri.toString())?.Source;
     }
 
     private createUri(sourceName: string): Uri {
         return Uri.parse(this.scheme + "://" +
             sourceName.replace(/\\/g, "/").replace(/(.*)\/(.*)/g, "$1/[metadata] $2"));
     }
-}
+}