Skip to content

Commit

Permalink
made reportMissingTypeStub diagnostics warning by default and some re…
Browse files Browse the repository at this point in the history
…factoring around code actions. (microsoft#507)

* enable generate type stub diagnostic (and hence code action) on by default as "warning"

* refactored code action code a bit
  • Loading branch information
heejaechang authored Feb 11, 2020
1 parent 9521693 commit b1e0ff8
Show file tree
Hide file tree
Showing 6 changed files with 114 additions and 98 deletions.
2 changes: 1 addition & 1 deletion docs/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ The following settings control pyright’s diagnostic output (warnings or errors

**reportMissingImports** [boolean or string, optional]: Generate or suppress diagnostics for imports that have no corresponding imported python file or type stub file. The default value for this setting is 'none', although pyright can do a much better job of static type checking if type stub files are provided for all imports.

**reportMissingTypeStubs** [boolean or string, optional]: Generate or suppress diagnostics for imports that have no corresponding type stub file (either a typeshed file or a custom type stub). The type checker requires type stubs to do its best job at analysis. The default value for this setting is 'none', although pyright can do a much better job of static type checking if type stub files are provided for all imports.
**reportMissingTypeStubs** [boolean or string, optional]: Generate or suppress diagnostics for imports that have no corresponding type stub file (either a typeshed file or a custom type stub). The type checker requires type stubs to do its best job at analysis. The default value for this setting is 'warning'. Note that there is a corresponding quick fix for this diagnostics that let you generate custom type stub to improve editing experiences.

**reportImportCycles** [boolean or string, optional]: Generate or suppress diagnostics for cyclical import chains. These are not errors in Python, but they do slow down type analysis and often hint at architectural layering issues. Generally, they should be avoided. The default value for this setting is 'none'. Note that there are import cycles in the typeshed stdlib typestub files that are ignored by this setting.

Expand Down
2 changes: 1 addition & 1 deletion server/src/common/configOptions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,7 @@ export function getDefaultDiagnosticSettings(): DiagnosticSettings {
enableTypeIgnoreComments: true,
reportTypeshedErrors: 'none',
reportMissingImports: 'error',
reportMissingTypeStubs: 'none',
reportMissingTypeStubs: 'warning',
reportImportCycles: 'none',
reportUnusedImport: 'none',
reportUnusedClass: 'none',
Expand Down
112 changes: 22 additions & 90 deletions server/src/languageServerBase.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,24 +5,20 @@
*/

import {
CodeAction, CodeActionKind, Command, ConfigurationItem,
createConnection, Diagnostic, DiagnosticRelatedInformation, DiagnosticSeverity,
DiagnosticTag, DocumentSymbol, ExecuteCommandParams, IConnection, InitializeResult,
IPCMessageReader, IPCMessageWriter, Location, MarkupKind, ParameterInformation,
RemoteConsole, SignatureInformation, SymbolInformation, TextDocuments, TextEdit, WorkspaceEdit
CodeAction, CodeActionKind, CodeActionParams, Command,
ConfigurationItem, createConnection, Diagnostic, DiagnosticRelatedInformation,
DiagnosticSeverity, DiagnosticTag, DocumentSymbol, ExecuteCommandParams, IConnection,
InitializeResult, IPCMessageReader, IPCMessageWriter, Location, MarkupKind,
ParameterInformation, RemoteConsole, SignatureInformation, SymbolInformation, TextDocuments, TextEdit, WorkspaceEdit
} from 'vscode-languageserver';

import { AnalyzerService } from './analyzer/service';
import { CommandController, ServerCommand } from './commands/commandController';
import { Commands } from './commands/commands';
import { CommandLineOptions } from './common/commandLineOptions';
import {
AddMissingOptionalToParamAction, CreateTypeStubFileAction,
Diagnostic as AnalyzerDiagnostic, DiagnosticCategory
} from './common/diagnostic';
import * as debug from './common/debug'
import { Diagnostic as AnalyzerDiagnostic, DiagnosticCategory } from './common/diagnostic';
import './common/extensions';
import { combinePaths, convertPathToUri, convertUriToPath, getDirectoryPath, normalizePath } from './common/pathUtils';
import { Position, Range } from './common/textRange';
import { combinePaths, convertPathToUri, convertUriToPath, normalizePath } from './common/pathUtils';
import { Position } from './common/textRange';
import { createFromRealFileSystem, VirtualFileSystem } from './common/vfs';
import { CompletionItemData } from './languageService/completionProvider';
import { WorkspaceMap } from './workspaceMap';
Expand Down Expand Up @@ -51,8 +47,6 @@ export abstract class LanguageServerBase {
// File system abstraction.
fs: VirtualFileSystem;

// Command controller.
private controller: ServerCommand;
// Create a simple text document manager. The text document manager
// supports full document sync only.
private _documents: TextDocuments = new TextDocuments();
Expand All @@ -62,16 +56,17 @@ export abstract class LanguageServerBase {
// Tracks whether we're currently displaying progress.
private _isDisplayingProgress = false;

constructor(private _productName: string, rootDirectory?: string) {
constructor(private _productName: string, rootDirectory: string) {
this.connection.console.log(`${ _productName } language server starting`);
// virtual file system to be used. initialized to real file system by default. but can't be overritten
this.fs = createFromRealFileSystem(this.connection.console);
// Stash the base directory into a global variable.
(global as any).__rootDirectory = rootDirectory ? rootDirectory : getDirectoryPath(__dirname);
debug.assertDefined(rootDirectory);
(global as any).__rootDirectory = rootDirectory;
this.connection.console.log(`Server root directory: ${ rootDirectory }`);

// Create workspace map.
this.workspaceMap = new WorkspaceMap(this);
// Create command controller.
this.controller = new CommandController(this);
// Make the text document manager listen on the connection
// for open, change and close text document events.
this._documents.listen(this.connection);
Expand All @@ -81,6 +76,8 @@ export abstract class LanguageServerBase {
this.connection.listen();
}

protected abstract async executeCommand(cmdParams: ExecuteCommandParams): Promise<any>;
protected abstract async executeCodeAction(cmdParams: CodeActionParams): Promise<(Command | CodeAction)[] | undefined | null>;
abstract async getSettings(workspace: WorkspaceServiceInstance): Promise<ServerSettings>;

// Provides access to logging to the client output window.
Expand Down Expand Up @@ -130,7 +127,7 @@ export abstract class LanguageServerBase {

const fileOrFiles = results.filesRequiringAnalysis !== 1 ? 'files' : 'file';
this.connection.sendNotification('pyright/reportProgress',
`${results.filesRequiringAnalysis} ${fileOrFiles} to analyze`);
`${ results.filesRequiringAnalysis } ${ fileOrFiles } to analyze`);
}
} else {
if (this._isDisplayingProgress) {
Expand Down Expand Up @@ -205,71 +202,10 @@ export abstract class LanguageServerBase {
this.updateSettingsForAllWorkspaces();
});

this.connection.onCodeAction(params => {
this._recordUserInteractionTime();

const sortImportsCodeAction = CodeAction.create(
'Organize Imports', Command.create('Organize Imports', Commands.orderImports),
CodeActionKind.SourceOrganizeImports);
const codeActions: CodeAction[] = [sortImportsCodeAction];

const filePath = convertUriToPath(params.textDocument.uri);
const workspace = this.workspaceMap.getWorkspaceForFile(filePath);
if (!workspace.disableLanguageServices) {
const range: Range = {
start: {
line: params.range.start.line,
character: params.range.start.character
},
end: {
line: params.range.end.line,
character: params.range.end.character
}
};

const diags = workspace.serviceInstance.getDiagnosticsForRange(filePath, range);
const typeStubDiag = diags.find(d => {
const actions = d.getActions();
return actions && actions.find(a => a.action === Commands.createTypeStub);
});

if (typeStubDiag) {
const action = typeStubDiag.getActions()!.find(
a => a.action === Commands.createTypeStub) as CreateTypeStubFileAction;
if (action) {
const createTypeStubAction = CodeAction.create(
`Create Type Stub For ‘${action.moduleName}’`,
Command.create('Create Type Stub', Commands.createTypeStub,
workspace.rootPath, action.moduleName),
CodeActionKind.QuickFix);
codeActions.push(createTypeStubAction);
}
}

const addOptionalDiag = diags.find(d => {
const actions = d.getActions();
return actions && actions.find(a => a.action === Commands.addMissingOptionalToParam);
});

if (addOptionalDiag) {
const action = addOptionalDiag.getActions()!.find(
a => a.action === Commands.addMissingOptionalToParam) as AddMissingOptionalToParamAction;
if (action) {
const addMissingOptionalAction = CodeAction.create(
`Add 'Optional' to type annotation`,
Command.create(`Add 'Optional' to type annotation`, Commands.addMissingOptionalToParam,
action.offsetOfTypeNode),
CodeActionKind.QuickFix);
codeActions.push(addMissingOptionalAction);
}
}
}

return codeActions;
});
this.connection.onCodeAction((params: CodeActionParams) => this.executeCodeAction(params));

this.connection.onDefinition(params => {
this._recordUserInteractionTime();
this.recordUserInteractionTime();

const filePath = convertUriToPath(params.textDocument.uri);

Expand Down Expand Up @@ -312,7 +248,7 @@ export abstract class LanguageServerBase {
});

this.connection.onDocumentSymbol(params => {
this._recordUserInteractionTime();
this.recordUserInteractionTime();

const filePath = convertUriToPath(params.textDocument.uri);

Expand Down Expand Up @@ -493,7 +429,7 @@ export abstract class LanguageServerBase {
});

this.connection.onDidChangeTextDocument(params => {
this._recordUserInteractionTime();
this.recordUserInteractionTime();

const filePath = convertUriToPath(params.textDocument.uri);
const service = this.workspaceMap.getWorkspaceForFile(filePath).serviceInstance;
Expand Down Expand Up @@ -534,10 +470,6 @@ export abstract class LanguageServerBase {
this.connection.onExecuteCommand((cmdParams: ExecuteCommandParams) => this.executeCommand(cmdParams));
}

protected executeCommand(cmdParams: ExecuteCommandParams): Promise<any> {
return this.controller.execute(cmdParams);
}

updateSettingsForAllWorkspaces(): void {
this.workspaceMap.forEach(workspace => {
this.updateSettingsForWorkspace(workspace).ignoreErrors();
Expand Down Expand Up @@ -640,7 +572,7 @@ export abstract class LanguageServerBase {
});
}

private _recordUserInteractionTime() {
protected recordUserInteractionTime() {
// Tell all of the services that the user is actively
// interacting with one or more editors, so they should
// back off from performing any work.
Expand Down
61 changes: 61 additions & 0 deletions server/src/languageService/codeActionProvider.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
/*
* codeActionProvider.ts
* Copyright (c) Microsoft Corporation.
* Licensed under the MIT license.
*/

import { CodeAction, CodeActionKind, Command } from 'vscode-languageserver';
import { Commands } from '../commands/commands';
import { AddMissingOptionalToParamAction, CreateTypeStubFileAction } from '../common/diagnostic';
import { Range } from '../common/textRange';
import { WorkspaceServiceInstance } from '../languageServerBase';

export class CodeActionProvider {
static getCodeActionsForPosition(workspace: WorkspaceServiceInstance, filePath: string, range: Range) {
const sortImportsCodeAction = CodeAction.create(
'Organize Imports', Command.create('Organize Imports', Commands.orderImports),
CodeActionKind.SourceOrganizeImports);
const codeActions: CodeAction[] = [sortImportsCodeAction];

if (!workspace.disableLanguageServices) {
const diags = workspace.serviceInstance.getDiagnosticsForRange(filePath, range);
const typeStubDiag = diags.find(d => {
const actions = d.getActions();
return actions && actions.find(a => a.action === Commands.createTypeStub);
});

if (typeStubDiag) {
const action = typeStubDiag.getActions()!.find(
a => a.action === Commands.createTypeStub) as CreateTypeStubFileAction;
if (action) {
const createTypeStubAction = CodeAction.create(
`Create Type Stub For ‘${ action.moduleName }’`,
Command.create('Create Type Stub', Commands.createTypeStub,
workspace.rootPath, action.moduleName),
CodeActionKind.QuickFix);
codeActions.push(createTypeStubAction);
}
}

const addOptionalDiag = diags.find(d => {
const actions = d.getActions();
return actions && actions.find(a => a.action === Commands.addMissingOptionalToParam);
});

if (addOptionalDiag) {
const action = addOptionalDiag.getActions()!.find(
a => a.action === Commands.addMissingOptionalToParam) as AddMissingOptionalToParamAction;
if (action) {
const addMissingOptionalAction = CodeAction.create(
`Add 'Optional' to type annotation`,
Command.create(`Add 'Optional' to type annotation`, Commands.addMissingOptionalToParam,
action.offsetOfTypeNode),
CodeActionKind.QuickFix);
codeActions.push(addMissingOptionalAction);
}
}
}

return codeActions;
}
}
2 changes: 1 addition & 1 deletion server/src/languageService/definitionProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@

import * as ParseTreeUtils from '../analyzer/parseTreeUtils';
import { TypeEvaluator } from '../analyzer/typeEvaluator';
import { Position, DocumentRange, rangesAreEqual } from '../common/textRange';
import { convertPositionToOffset } from '../common/positionUtils';
import { DocumentRange, Position, rangesAreEqual } from '../common/textRange';
import { ParseNodeType } from '../parser/parseNodes';
import { ParseResults } from '../parser/parser';

Expand Down
33 changes: 28 additions & 5 deletions server/src/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,28 +4,39 @@
* Implements pyright language server.
*/

import * as fs from 'fs';
import * as path from 'path';
import { isArray } from 'util';
import { CodeAction, CodeActionParams, Command, ExecuteCommandParams } from 'vscode-languageserver';
import { CommandController } from './commands/commandController';
import * as debug from './common/debug';
import { convertUriToPath, getDirectoryPath, normalizeSlashes } from './common/pathUtils';
import { LanguageServerBase, ServerSettings, WorkspaceServiceInstance } from './languageServerBase';
import { CodeActionProvider } from './languageService/codeActionProvider';

class Server extends LanguageServerBase {
private _controller: CommandController;

constructor() {
super('Pyright');
debug.assert(fs.existsSync(path.join(__dirname, 'typeshed-fallback')), 'Unable to locate typeshed fallback folder.');
super('Pyright', getDirectoryPath(__dirname));
this._controller = new CommandController(this);
}

async getSettings(workspace: WorkspaceServiceInstance): Promise<ServerSettings> {
const serverSettings: ServerSettings = {};
try {
const pythonSection = await this.getConfiguration(workspace, 'python');
if (pythonSection) {
serverSettings.pythonPath = pythonSection.pythonPath;
serverSettings.venvPath = pythonSection.venvPath;
serverSettings.pythonPath = normalizeSlashes(pythonSection.pythonPath);
serverSettings.venvPath = normalizeSlashes(pythonSection.venvPath);
}

const pythonAnalysisSection = await this.getConfiguration(workspace, 'python.analysis');
if (pythonAnalysisSection) {
const typeshedPaths = pythonAnalysisSection.typeshedPaths;
if (typeshedPaths && isArray(typeshedPaths) && typeshedPaths.length > 0) {
serverSettings.typeshedPath = typeshedPaths[0];
serverSettings.typeshedPath = normalizeSlashes(typeshedPaths[0]);
}
}

Expand All @@ -40,10 +51,22 @@ class Server extends LanguageServerBase {
serverSettings.disableLanguageServices = false;
}
} catch (error) {
this.console.log(`Error reading settings: ${error}`);
this.console.log(`Error reading settings: ${ error }`);
}
return serverSettings;
}

protected executeCommand(cmdParams: ExecuteCommandParams): Promise<any> {
return this._controller.execute(cmdParams);
}

protected async executeCodeAction(params: CodeActionParams): Promise<(Command | CodeAction)[] | undefined | null> {
this.recordUserInteractionTime();

const filePath = convertUriToPath(params.textDocument.uri);
const workspace = this.workspaceMap.getWorkspaceForFile(filePath);
return CodeActionProvider.getCodeActionsForPosition(workspace, filePath, params.range);
}
}

export const server = new Server();

0 comments on commit b1e0ff8

Please sign in to comment.