From 60fdb699c0732a0fda1077437db2a63227e8ba45 Mon Sep 17 00:00:00 2001 From: detachhead Date: Thu, 22 Aug 2024 21:22:42 +1000 Subject: [PATCH 01/28] add `--writebaseline` cli arg --- docs/command-line.md | 1 + packages/pyright-internal/src/baseline.ts | 49 +++++++++++++++++++++++ packages/pyright-internal/src/pyright.ts | 10 +++++ 3 files changed, 60 insertions(+) create mode 100644 packages/pyright-internal/src/baseline.ts diff --git a/docs/command-line.md b/docs/command-line.md index 6fc5cada2..943f73764 100644 --- a/docs/command-line.md +++ b/docs/command-line.md @@ -13,6 +13,7 @@ Pyright can be run as either a VS Code extension or as a command-line tool. The | --level | Minimum diagnostic level (error or warning) | | --outputjson | Output results in JSON format | | --gitlabcodequality | Output results to a gitlab code quality report | +| --writebaseline | Write new errors to the baseline file | | -p, --project `` | Use the configuration file at this location | | --pythonpath `` | Path to the Python interpreter (2) | | --pythonplatform `` | Analyze for platform (Darwin, Linux, Windows) | diff --git a/packages/pyright-internal/src/baseline.ts b/packages/pyright-internal/src/baseline.ts new file mode 100644 index 000000000..9c3ccefcc --- /dev/null +++ b/packages/pyright-internal/src/baseline.ts @@ -0,0 +1,49 @@ +import { DiagnosticRule } from './common/diagnosticRules'; +import { FileDiagnostics } from './common/diagnosticSink'; +import { Range } from './common/textRange'; +import { mkdirSync, writeFileSync } from 'fs'; +import { Uri } from './common/uri/uri'; + +interface BaselineFile { + files: { + [filePath: string]: { + code: DiagnosticRule | undefined; + message: string; + range: Range; + }[]; + }; +} + +const diagnosticsToBaseline = (rootDir: Uri, filesWithDiagnostics: FileDiagnostics[]): BaselineFile => { + const baselineData: BaselineFile = { + files: {}, + }; + for (const fileWithDiagnostics of filesWithDiagnostics) { + const filePath = rootDir.getRelativePath(fileWithDiagnostics.fileUri)!.toString(); + if (!fileWithDiagnostics.diagnostics.length) { + continue; + } + if (!(filePath in baselineData.files)) { + baselineData.files[filePath] = []; + } + baselineData.files[filePath].push( + ...fileWithDiagnostics.diagnostics.map((diagnostic) => ({ + code: diagnostic.getRule() as DiagnosticRule | undefined, + message: diagnostic.message, + range: diagnostic.range, + })) + ); + } + return baselineData; +}; + +export const writeBaseline = async ( + rootDir: Uri, + baselineFilePath: string, + filesWithDiagnostics: FileDiagnostics[] +) => { + const baselineData = diagnosticsToBaseline(rootDir, filesWithDiagnostics); + const baselineFile = rootDir.combinePaths(baselineFilePath); + mkdirSync(baselineFile.getDirectory().getPath(), { recursive: true }); + writeFileSync(baselineFile.getPath(), JSON.stringify(baselineData, undefined, 4)); +}; diff --git a/packages/pyright-internal/src/pyright.ts b/packages/pyright-internal/src/pyright.ts index a9b27a431..5ec81d60e 100644 --- a/packages/pyright-internal/src/pyright.ts +++ b/packages/pyright-internal/src/pyright.ts @@ -48,6 +48,7 @@ import * as core from '@actions/core'; import * as command from '@actions/core/lib/command'; import { convertDiagnostics } from 'pyright-to-gitlab-ci/src/converter'; import path from 'path'; +import { writeBaseline } from './baseline'; type SeverityLevel = 'error' | 'warning' | 'information'; @@ -159,6 +160,7 @@ async function processArgs(): Promise { { name: 'level', type: String }, { name: 'outputjson', type: Boolean }, { name: 'gitlabcodequality', type: String }, + { name: 'writebaseline', type: Boolean }, { name: 'project', alias: 'p', type: String }, { name: 'pythonpath', type: String }, { name: 'pythonplatform', type: String }, @@ -506,6 +508,13 @@ async function runSingleThreaded( ) ); } + if (args.writebaseline) { + const rootDir = + typeof options.executionRoot === 'string' || options.executionRoot === undefined + ? Uri.file(options.executionRoot ?? '', service.serviceProvider) + : options.executionRoot; + writeBaseline(rootDir, '.basedpyright/baseline.json', results.diagnostics); + } errorCount += report.errorCount; if (treatWarningsAsErrors) { errorCount += report.warningCount; @@ -1129,6 +1138,7 @@ function printUsage() { ' --level Minimum diagnostic level (error or warning)\n' + ' --outputjson Output results in JSON format\n' + ' --gitlabcodequality Output results to a gitlab code quality report\n' + + ' --writebaseline Write new errors to the baseline file\n' + ' -p,--project Use the configuration file at this location\n' + ' --pythonplatform Analyze for a specific platform (Darwin, Linux, Windows)\n' + ' --pythonpath Path to the Python interpreter\n' + From 4924e601cef5b9120b0e66a5bc41e9a739279da9 Mon Sep 17 00:00:00 2001 From: detachhead Date: Sat, 24 Aug 2024 23:11:13 +1000 Subject: [PATCH 02/28] baseline matching --- packages/pyright-internal/src/baseline.ts | 40 +++++++++++++++++++---- packages/pyright-internal/src/pyright.ts | 17 +++++----- 2 files changed, 42 insertions(+), 15 deletions(-) diff --git a/packages/pyright-internal/src/baseline.ts b/packages/pyright-internal/src/baseline.ts index 9c3ccefcc..841094de7 100644 --- a/packages/pyright-internal/src/baseline.ts +++ b/packages/pyright-internal/src/baseline.ts @@ -1,8 +1,9 @@ import { DiagnosticRule } from './common/diagnosticRules'; import { FileDiagnostics } from './common/diagnosticSink'; import { Range } from './common/textRange'; -import { mkdirSync, writeFileSync } from 'fs'; +import { mkdirSync, readFileSync, writeFileSync } from 'fs'; import { Uri } from './common/uri/uri'; +// import { Diagnostic } from './common/diagnostic'; interface BaselineFile { files: { @@ -14,6 +15,8 @@ interface BaselineFile { }; } +const baselineFilePath = (rootDir: Uri) => rootDir.combinePaths('.basedpyright/baseline.json'); + const diagnosticsToBaseline = (rootDir: Uri, filesWithDiagnostics: FileDiagnostics[]): BaselineFile => { const baselineData: BaselineFile = { files: {}, @@ -37,13 +40,36 @@ const diagnosticsToBaseline = (rootDir: Uri, filesWithDiagnostics: FileDiagnosti return baselineData; }; -export const writeBaseline = async ( - rootDir: Uri, - baselineFilePath: string, - filesWithDiagnostics: FileDiagnostics[] -) => { +export const writeBaseline = async (rootDir: Uri, filesWithDiagnostics: FileDiagnostics[]) => { const baselineData = diagnosticsToBaseline(rootDir, filesWithDiagnostics); - const baselineFile = rootDir.combinePaths(baselineFilePath); + const baselineFile = baselineFilePath(rootDir); mkdirSync(baselineFile.getDirectory().getPath(), { recursive: true }); writeFileSync(baselineFile.getPath(), JSON.stringify(baselineData, undefined, 4)); }; + +export const getBaselinedErrors = (rootDir: Uri): BaselineFile => + JSON.parse(readFileSync(baselineFilePath(rootDir).getPath(), 'utf8')); + +export const filterOutBaselinedDiagnostics = (rootDir: Uri, filesWithDiagnostics: FileDiagnostics[]): void => { + const baselineFile = getBaselinedErrors(rootDir); + for (const fileWithDiagnostics of filesWithDiagnostics) { + const newDiagnostics = []; + const baselinedErrorsForFile = + baselineFile.files[rootDir.getRelativePath(fileWithDiagnostics.fileUri)!.toString()]; + for (const diagnostic of fileWithDiagnostics.diagnostics) { + const matchedIndex = baselinedErrorsForFile.findIndex( + (baselinedError) => + baselinedError.message === diagnostic.message && + baselinedError.code === diagnostic.getRule() && + baselinedError.range.start.character === diagnostic.range.start.character && + baselinedError.range.end.character === diagnostic.range.end.character + ); + if (matchedIndex >= 0) { + baselinedErrorsForFile.splice(matchedIndex, 1); + } else { + newDiagnostics.push(diagnostic); + } + } + fileWithDiagnostics.diagnostics = newDiagnostics; + } +}; diff --git a/packages/pyright-internal/src/pyright.ts b/packages/pyright-internal/src/pyright.ts index 5ec81d60e..115f6ae45 100644 --- a/packages/pyright-internal/src/pyright.ts +++ b/packages/pyright-internal/src/pyright.ts @@ -48,7 +48,7 @@ import * as core from '@actions/core'; import * as command from '@actions/core/lib/command'; import { convertDiagnostics } from 'pyright-to-gitlab-ci/src/converter'; import path from 'path'; -import { writeBaseline } from './baseline'; +import { filterOutBaselinedDiagnostics, writeBaseline } from './baseline'; type SeverityLevel = 'error' | 'warning' | 'information'; @@ -474,6 +474,14 @@ async function runSingleThreaded( return; } + const rootDir = + typeof options.executionRoot === 'string' || options.executionRoot === undefined + ? Uri.file(options.executionRoot ?? '', service.serviceProvider) + : options.executionRoot; + if (args.writebaseline) { + writeBaseline(rootDir, results.diagnostics); + } + filterOutBaselinedDiagnostics(rootDir, results.diagnostics); let errorCount = 0; if (!args.createstub && !args.verifytypes) { let report: DiagnosticResult; @@ -508,13 +516,6 @@ async function runSingleThreaded( ) ); } - if (args.writebaseline) { - const rootDir = - typeof options.executionRoot === 'string' || options.executionRoot === undefined - ? Uri.file(options.executionRoot ?? '', service.serviceProvider) - : options.executionRoot; - writeBaseline(rootDir, '.basedpyright/baseline.json', results.diagnostics); - } errorCount += report.errorCount; if (treatWarningsAsErrors) { errorCount += report.warningCount; From 839b63a9abc3daac62605a9be34db2d1db7b3095 Mon Sep 17 00:00:00 2001 From: detachhead Date: Sun, 25 Aug 2024 19:18:54 +1000 Subject: [PATCH 03/28] support baseline in the language server & vscode extension --- packages/pyright-internal/src/baseline.ts | 18 +++-- .../src/commands/commandController.ts | 6 ++ .../pyright-internal/src/commands/commands.ts | 1 + .../src/commands/writeBaseline.ts | 19 +++++ .../src/common/languageServerInterface.ts | 2 + .../src/languageServerBase.ts | 72 +++++++++++-------- .../harness/fourslash/testLanguageService.ts | 2 + packages/vscode-pyright/package.json | 5 ++ packages/vscode-pyright/src/extension.ts | 2 +- 9 files changed, 92 insertions(+), 35 deletions(-) create mode 100644 packages/pyright-internal/src/commands/writeBaseline.ts diff --git a/packages/pyright-internal/src/baseline.ts b/packages/pyright-internal/src/baseline.ts index 841094de7..fd1cc10d4 100644 --- a/packages/pyright-internal/src/baseline.ts +++ b/packages/pyright-internal/src/baseline.ts @@ -3,7 +3,6 @@ import { FileDiagnostics } from './common/diagnosticSink'; import { Range } from './common/textRange'; import { mkdirSync, readFileSync, writeFileSync } from 'fs'; import { Uri } from './common/uri/uri'; -// import { Diagnostic } from './common/diagnostic'; interface BaselineFile { files: { @@ -17,7 +16,7 @@ interface BaselineFile { const baselineFilePath = (rootDir: Uri) => rootDir.combinePaths('.basedpyright/baseline.json'); -const diagnosticsToBaseline = (rootDir: Uri, filesWithDiagnostics: FileDiagnostics[]): BaselineFile => { +export const diagnosticsToBaseline = (rootDir: Uri, filesWithDiagnostics: FileDiagnostics[]): BaselineFile => { const baselineData: BaselineFile = { files: {}, }; @@ -47,8 +46,16 @@ export const writeBaseline = async (rootDir: Uri, filesWithDiagnostics: FileDiag writeFileSync(baselineFile.getPath(), JSON.stringify(baselineData, undefined, 4)); }; -export const getBaselinedErrors = (rootDir: Uri): BaselineFile => - JSON.parse(readFileSync(baselineFilePath(rootDir).getPath(), 'utf8')); +export const getBaselinedErrors = (rootDir: Uri): BaselineFile => { + const path = baselineFilePath(rootDir).getPath(); + let baselineFileContents; + try { + baselineFileContents = readFileSync(path, 'utf8'); + } catch (e) { + return { files: {} }; + } + return JSON.parse(baselineFileContents); +}; export const filterOutBaselinedDiagnostics = (rootDir: Uri, filesWithDiagnostics: FileDiagnostics[]): void => { const baselineFile = getBaselinedErrors(rootDir); @@ -56,6 +63,9 @@ export const filterOutBaselinedDiagnostics = (rootDir: Uri, filesWithDiagnostics const newDiagnostics = []; const baselinedErrorsForFile = baselineFile.files[rootDir.getRelativePath(fileWithDiagnostics.fileUri)!.toString()]; + if (!baselinedErrorsForFile) { + continue; + } for (const diagnostic of fileWithDiagnostics.diagnostics) { const matchedIndex = baselinedErrorsForFile.findIndex( (baselinedError) => diff --git a/packages/pyright-internal/src/commands/commandController.ts b/packages/pyright-internal/src/commands/commandController.ts index 390ab03d0..03a65403e 100644 --- a/packages/pyright-internal/src/commands/commandController.ts +++ b/packages/pyright-internal/src/commands/commandController.ts @@ -14,6 +14,7 @@ import { CreateTypeStubCommand } from './createTypeStub'; import { DumpFileDebugInfoCommand } from './dumpFileDebugInfoCommand'; import { QuickActionCommand } from './quickActionCommand'; import { RestartServerCommand } from './restartServer'; +import { WriteBaselineCommand } from './writeBaseline'; export interface ServerCommand { execute(cmdParams: ExecuteCommandParams, token: CancellationToken): Promise; @@ -24,12 +25,14 @@ export class CommandController implements ServerCommand { private _restartServer: RestartServerCommand; private _quickAction: QuickActionCommand; private _dumpFileDebugInfo: DumpFileDebugInfoCommand; + private _writeBaseline: WriteBaselineCommand; constructor(ls: LanguageServerInterface) { this._createStub = new CreateTypeStubCommand(ls); this._restartServer = new RestartServerCommand(ls); this._quickAction = new QuickActionCommand(ls); this._dumpFileDebugInfo = new DumpFileDebugInfoCommand(ls); + this._writeBaseline = new WriteBaselineCommand(ls); } async execute(cmdParams: ExecuteCommandParams, token: CancellationToken): Promise { @@ -50,6 +53,9 @@ export class CommandController implements ServerCommand { return this._dumpFileDebugInfo.execute(cmdParams, token); } + case Commands.writeBaseline: { + return this._writeBaseline.execute(); + } default: { return new ResponseError(1, 'Unsupported command'); } diff --git a/packages/pyright-internal/src/commands/commands.ts b/packages/pyright-internal/src/commands/commands.ts index 56289c36b..b13ae86f1 100644 --- a/packages/pyright-internal/src/commands/commands.ts +++ b/packages/pyright-internal/src/commands/commands.ts @@ -19,4 +19,5 @@ export const enum Commands { dumpCachedTypes = 'basedpyright.dumpCachedTypes', dumpCodeFlowGraph = 'basedpyright.dumpCodeFlowGraph', import = 'basedpyright.import', + writeBaseline = 'basedpyright.writeBaseline', } diff --git a/packages/pyright-internal/src/commands/writeBaseline.ts b/packages/pyright-internal/src/commands/writeBaseline.ts new file mode 100644 index 000000000..e18f876f8 --- /dev/null +++ b/packages/pyright-internal/src/commands/writeBaseline.ts @@ -0,0 +1,19 @@ +import { ServerCommand } from './commandController'; +import { writeBaseline } from '../baseline'; +import { LanguageServerInterface } from '../common/languageServerInterface'; + +export class WriteBaselineCommand implements ServerCommand { + constructor(private _ls: LanguageServerInterface) { + // Empty + } + + async execute(): Promise { + // TODO: figure out a better way to get workspace root + const workspaceRoot = ( + await this._ls.getWorkspaceForFile( + this._ls.documentsWithDiagnostics[Object.keys(this._ls.documentsWithDiagnostics)[0]].fileUri + ) + ).rootUri!; + return writeBaseline(workspaceRoot, Object.values(this._ls.documentsWithDiagnostics)); + } +} diff --git a/packages/pyright-internal/src/common/languageServerInterface.ts b/packages/pyright-internal/src/common/languageServerInterface.ts index 24937d810..ea58a3e90 100644 --- a/packages/pyright-internal/src/common/languageServerInterface.ts +++ b/packages/pyright-internal/src/common/languageServerInterface.ts @@ -19,6 +19,7 @@ import { FileSystem } from './fileSystem'; import { FileWatcherHandler } from './fileWatcher'; import { ServiceProvider } from './serviceProvider'; import { Uri } from './uri/uri'; +import { FileDiagnostics } from './diagnosticSink'; export interface ServerSettings { venvPath?: Uri | undefined; @@ -136,6 +137,7 @@ export interface LanguageServerBaseInterface { export interface LanguageServerInterface extends LanguageServerBaseInterface { getWorkspaceForFile(fileUri: Uri, pythonPath?: Uri): Promise; + readonly documentsWithDiagnostics: Record; } export interface WindowService extends WindowInterface { diff --git a/packages/pyright-internal/src/languageServerBase.ts b/packages/pyright-internal/src/languageServerBase.ts index 0c350654e..d5d62c95a 100644 --- a/packages/pyright-internal/src/languageServerBase.ts +++ b/packages/pyright-internal/src/languageServerBase.ts @@ -67,6 +67,7 @@ import { TextDocumentEdit, TextDocumentPositionParams, TextDocumentSyncKind, + WillSaveTextDocumentParams, WorkDoneProgressReporter, WorkspaceEdit, WorkspaceFoldersChangeEvent, @@ -133,6 +134,7 @@ import { InitStatus, WellKnownWorkspaceKinds, Workspace, WorkspaceFactory } from import { githubRepo } from './constants'; import { SemanticTokensProvider, SemanticTokensProviderLegend } from './languageService/semanticTokensProvider'; import { RenameUsageFinder } from './analyzer/renameUsageFinder'; +import { diagnosticsToBaseline, filterOutBaselinedDiagnostics, getBaselinedErrors } from './baseline'; export abstract class LanguageServerBase implements LanguageServerInterface, Disposable { // We support running only one "find all reference" at a time. @@ -180,7 +182,7 @@ export abstract class LanguageServerBase implements LanguageServerInterface, Dis protected readonly caseSensitiveDetector: CaseSensitivityDetector; // The URIs for which diagnostics are reported - protected readonly documentsWithDiagnostics = new Set(); + readonly documentsWithDiagnostics: Record = {}; protected readonly dynamicFeatures = new DynamicFeatures(); @@ -505,6 +507,7 @@ export abstract class LanguageServerBase implements LanguageServerInterface, Dis this.connection.onDidCloseTextDocument(async (params) => this.onDidCloseTextDocument(params)); this.connection.onDidChangeWatchedFiles((params) => this.onDidChangeWatchedFiles(params)); this.connection.workspace.onWillRenameFiles(this.onRenameFiles); + this.connection.onWillSaveTextDocument(this.onSaveTextDocument); this.connection.onExecuteCommand(async (params, token, reporter) => this.onExecuteCommand(params, token, reporter) ); @@ -1152,6 +1155,17 @@ export abstract class LanguageServerBase implements LanguageServerInterface, Dis return result; }; + protected onSaveTextDocument = (params: WillSaveTextDocumentParams) => { + const baselineFile = getBaselinedErrors(this.serverOptions.rootDirectory); + const fileKey = this.serverOptions.rootDirectory + .getRelativePath(Uri.file(params.textDocument.uri, this.serviceProvider))! + .toString(); + //cringe + baselineFile.files[fileKey] = diagnosticsToBaseline(this.serverOptions.rootDirectory, [ + this.documentsWithDiagnostics[params.textDocument.uri.toString()], + ]).files[fileKey]; + }; + protected async onExecuteCommand( params: ExecuteCommandParams, token: CancellationToken, @@ -1218,14 +1232,17 @@ export abstract class LanguageServerBase implements LanguageServerInterface, Dis return Promise.resolve(); } - protected convertDiagnostics(fs: FileSystem, fileDiagnostics: FileDiagnostics): PublishDiagnosticsParams[] { - return [ - { - uri: convertUriToLspUriString(fs, fileDiagnostics.fileUri), - version: fileDiagnostics.version, - diagnostics: this._convertDiagnostics(fs, fileDiagnostics.diagnostics), - }, - ]; + protected async convertDiagnostics( + fs: FileSystem, + fileDiagnostics: FileDiagnostics + ): Promise { + const workspace = await this.getWorkspaceForFile(fileDiagnostics.fileUri); + filterOutBaselinedDiagnostics(workspace.rootUri!, [fileDiagnostics]); + return { + uri: convertUriToLspUriString(fs, fileDiagnostics.fileUri), + version: fileDiagnostics.version, + diagnostics: this._convertDiagnostics(fs, fileDiagnostics.diagnostics), + }; } protected getDiagCode(_diag: AnalyzerDiagnostic, rule: string | undefined): string | undefined { @@ -1239,7 +1256,7 @@ export abstract class LanguageServerBase implements LanguageServerInterface, Dis return; } - this.sendDiagnostics(this.convertDiagnostics(fs, fileDiag)); + this.sendDiagnostics(fs, fileDiag); }); results.configParseErrors.forEach((error) => @@ -1291,23 +1308,19 @@ export abstract class LanguageServerBase implements LanguageServerInterface, Dis } protected onWorkspaceRemoved(workspace: Workspace) { - const documentsWithDiagnosticsList = [...this.documentsWithDiagnostics]; const otherWorkspaces = this.workspaceFactory.items().filter((w) => w !== workspace); - for (const uri of documentsWithDiagnosticsList) { - const fileUri = this.convertLspUriStringToUri(uri); - - if (workspace.service.isTracked(fileUri)) { + for (const fileWithDiagnostics of Object.values(this.documentsWithDiagnostics)) { + if (workspace.service.isTracked(fileWithDiagnostics.fileUri)) { // Do not clean up diagnostics for files tracked by multiple workspaces - if (otherWorkspaces.some((w) => w.service.isTracked(fileUri))) { + if (otherWorkspaces.some((w) => w.service.isTracked(fileWithDiagnostics.fileUri))) { continue; } - this.sendDiagnostics([ - { - uri: uri, - diagnostics: [], - }, - ]); + this.sendDiagnostics(this.fs, { + fileUri: fileWithDiagnostics.fileUri, + diagnostics: fileWithDiagnostics.diagnostics, + version: undefined, + }); } } } @@ -1371,15 +1384,14 @@ export abstract class LanguageServerBase implements LanguageServerInterface, Dis }; } - protected sendDiagnostics(params: PublishDiagnosticsParams[]) { - for (const param of params) { - if (param.diagnostics.length === 0) { - this.documentsWithDiagnostics.delete(param.uri); - } else { - this.documentsWithDiagnostics.add(param.uri); - } - this.connection.sendDiagnostics(param); + protected async sendDiagnostics(fs: FileSystem, fileWithDiagnostics: FileDiagnostics) { + const key = fileWithDiagnostics.fileUri.toString(); + if (fileWithDiagnostics.diagnostics.length === 0) { + delete this.documentsWithDiagnostics[key]; + } else { + this.documentsWithDiagnostics[key] = fileWithDiagnostics; } + this.connection.sendDiagnostics(await this.convertDiagnostics(fs, fileWithDiagnostics)); } protected convertLspUriStringToUri(uri: string) { diff --git a/packages/pyright-internal/src/tests/harness/fourslash/testLanguageService.ts b/packages/pyright-internal/src/tests/harness/fourslash/testLanguageService.ts index e70e052ca..f93914ff0 100644 --- a/packages/pyright-internal/src/tests/harness/fourslash/testLanguageService.ts +++ b/packages/pyright-internal/src/tests/harness/fourslash/testLanguageService.ts @@ -34,6 +34,7 @@ import { CodeActionProvider } from '../../../languageService/codeActionProvider' import { WellKnownWorkspaceKinds, Workspace, createInitStatus } from '../../../workspaceFactory'; import { TestAccessHost } from '../testAccessHost'; import { HostSpecificFeatures } from './testState'; +import { FileDiagnostics } from '../../../common/diagnosticSink'; export class TestFeatures implements HostSpecificFeatures { importResolverFactory: ImportResolverFactory = AnalyzerService.createImportResolver; @@ -77,6 +78,7 @@ export class TestLanguageService implements LanguageServerInterface { readonly window = new TestWindow(); readonly supportAdvancedEdits = true; readonly serviceProvider: ServiceProvider; + readonly documentsWithDiagnostics: Record = {}; private readonly _workspace: Workspace; private readonly _defaultWorkspace: Workspace; diff --git a/packages/vscode-pyright/package.json b/packages/vscode-pyright/package.json index 6e7001ab5..1071f8a5b 100644 --- a/packages/vscode-pyright/package.json +++ b/packages/vscode-pyright/package.json @@ -82,6 +82,11 @@ "title": "Dump code flow graph for node ...", "category": "Pyright", "enablement": "editorLangId == python && pyright.development" + }, + { + "command": "basedpyright.writeBaseline", + "title": "Write new errors to baseline", + "category": "Pyright" } ], "menus": { diff --git a/packages/vscode-pyright/src/extension.ts b/packages/vscode-pyright/src/extension.ts index c8ba06717..727b72c49 100644 --- a/packages/vscode-pyright/src/extension.ts +++ b/packages/vscode-pyright/src/extension.ts @@ -294,7 +294,7 @@ export async function activate(context: ExtensionContext) { ); }); - const genericCommands = [Commands.createTypeStub, Commands.restartServer]; + const genericCommands = [Commands.createTypeStub, Commands.restartServer, Commands.writeBaseline]; genericCommands.forEach((command) => { context.subscriptions.push( commands.registerCommand(command, (...args: any[]) => { From 100f4b00a9efb6d774530ffb7bdd6fc6f4b2e2e7 Mon Sep 17 00:00:00 2001 From: detachhead Date: Tue, 27 Aug 2024 18:50:16 +1000 Subject: [PATCH 04/28] fix new errors being automatically written to the baseline --- packages/pyright-internal/src/baseline.ts | 55 +++++++++++-------- .../src/commands/writeBaseline.ts | 5 +- .../src/languageServerBase.ts | 46 +++++++++++----- packages/pyright-internal/src/pyright.ts | 6 +- 4 files changed, 69 insertions(+), 43 deletions(-) diff --git a/packages/pyright-internal/src/baseline.ts b/packages/pyright-internal/src/baseline.ts index fd1cc10d4..202fb7e49 100644 --- a/packages/pyright-internal/src/baseline.ts +++ b/packages/pyright-internal/src/baseline.ts @@ -14,7 +14,7 @@ interface BaselineFile { }; } -const baselineFilePath = (rootDir: Uri) => rootDir.combinePaths('.basedpyright/baseline.json'); +export const baselineFilePath = (rootDir: Uri) => rootDir.combinePaths('.basedpyright/baseline.json'); export const diagnosticsToBaseline = (rootDir: Uri, filesWithDiagnostics: FileDiagnostics[]): BaselineFile => { const baselineData: BaselineFile = { @@ -39,13 +39,17 @@ export const diagnosticsToBaseline = (rootDir: Uri, filesWithDiagnostics: FileDi return baselineData; }; -export const writeBaseline = async (rootDir: Uri, filesWithDiagnostics: FileDiagnostics[]) => { - const baselineData = diagnosticsToBaseline(rootDir, filesWithDiagnostics); +export const writeBaselineFile = (rootDir: Uri, baselineData: BaselineFile) => { const baselineFile = baselineFilePath(rootDir); mkdirSync(baselineFile.getDirectory().getPath(), { recursive: true }); writeFileSync(baselineFile.getPath(), JSON.stringify(baselineData, undefined, 4)); }; +export const writeDiagnosticsToBaselineFile = async (rootDir: Uri, filesWithDiagnostics: FileDiagnostics[]) => { + const baselineData = diagnosticsToBaseline(rootDir, filesWithDiagnostics); + writeBaselineFile(rootDir, baselineData); +}; + export const getBaselinedErrors = (rootDir: Uri): BaselineFile => { const path = baselineFilePath(rootDir).getPath(); let baselineFileContents; @@ -57,29 +61,34 @@ export const getBaselinedErrors = (rootDir: Uri): BaselineFile => { return JSON.parse(baselineFileContents); }; -export const filterOutBaselinedDiagnostics = (rootDir: Uri, filesWithDiagnostics: FileDiagnostics[]): void => { +export const filterOutBaselinedDiagnostics = ( + rootDir: Uri, + filesWithDiagnostics: readonly FileDiagnostics[] +): FileDiagnostics[] => { const baselineFile = getBaselinedErrors(rootDir); - for (const fileWithDiagnostics of filesWithDiagnostics) { - const newDiagnostics = []; + return filesWithDiagnostics.map((fileWithDiagnostics) => { const baselinedErrorsForFile = baselineFile.files[rootDir.getRelativePath(fileWithDiagnostics.fileUri)!.toString()]; if (!baselinedErrorsForFile) { - continue; + return fileWithDiagnostics; } - for (const diagnostic of fileWithDiagnostics.diagnostics) { - const matchedIndex = baselinedErrorsForFile.findIndex( - (baselinedError) => - baselinedError.message === diagnostic.message && - baselinedError.code === diagnostic.getRule() && - baselinedError.range.start.character === diagnostic.range.start.character && - baselinedError.range.end.character === diagnostic.range.end.character - ); - if (matchedIndex >= 0) { - baselinedErrorsForFile.splice(matchedIndex, 1); - } else { - newDiagnostics.push(diagnostic); - } - } - fileWithDiagnostics.diagnostics = newDiagnostics; - } + return { + ...fileWithDiagnostics, + diagnostics: fileWithDiagnostics.diagnostics.filter((diagnostic) => { + const matchedIndex = baselinedErrorsForFile.findIndex( + (baselinedError) => + baselinedError.message === diagnostic.message && + baselinedError.code === diagnostic.getRule() && + baselinedError.range.start.character === diagnostic.range.start.character && + baselinedError.range.end.character === diagnostic.range.end.character + ); + if (matchedIndex >= 0) { + baselinedErrorsForFile.splice(matchedIndex, 1); + return false; + } else { + return true; + } + }), + }; + }); }; diff --git a/packages/pyright-internal/src/commands/writeBaseline.ts b/packages/pyright-internal/src/commands/writeBaseline.ts index e18f876f8..d339fd054 100644 --- a/packages/pyright-internal/src/commands/writeBaseline.ts +++ b/packages/pyright-internal/src/commands/writeBaseline.ts @@ -1,5 +1,5 @@ import { ServerCommand } from './commandController'; -import { writeBaseline } from '../baseline'; +import { writeDiagnosticsToBaselineFile } from '../baseline'; import { LanguageServerInterface } from '../common/languageServerInterface'; export class WriteBaselineCommand implements ServerCommand { @@ -14,6 +14,7 @@ export class WriteBaselineCommand implements ServerCommand { this._ls.documentsWithDiagnostics[Object.keys(this._ls.documentsWithDiagnostics)[0]].fileUri ) ).rootUri!; - return writeBaseline(workspaceRoot, Object.values(this._ls.documentsWithDiagnostics)); + // TODO: this very likely will delete any baselined errors from files that aren't open. FIX BEFORE MERGE!!!!! + return writeDiagnosticsToBaselineFile(workspaceRoot, Object.values(this._ls.documentsWithDiagnostics)); } } diff --git a/packages/pyright-internal/src/languageServerBase.ts b/packages/pyright-internal/src/languageServerBase.ts index d5d62c95a..ca4fc5b86 100644 --- a/packages/pyright-internal/src/languageServerBase.ts +++ b/packages/pyright-internal/src/languageServerBase.ts @@ -134,7 +134,12 @@ import { InitStatus, WellKnownWorkspaceKinds, Workspace, WorkspaceFactory } from import { githubRepo } from './constants'; import { SemanticTokensProvider, SemanticTokensProviderLegend } from './languageService/semanticTokensProvider'; import { RenameUsageFinder } from './analyzer/renameUsageFinder'; -import { diagnosticsToBaseline, filterOutBaselinedDiagnostics, getBaselinedErrors } from './baseline'; +import { + diagnosticsToBaseline, + filterOutBaselinedDiagnostics, + getBaselinedErrors, + writeBaselineFile, +} from './baseline'; export abstract class LanguageServerBase implements LanguageServerInterface, Disposable { // We support running only one "find all reference" at a time. @@ -583,7 +588,7 @@ export abstract class LanguageServerBase implements LanguageServerInterface, Dis } const result: InitializeResult = { capabilities: { - textDocumentSync: TextDocumentSyncKind.Incremental, + textDocumentSync: { willSave: true, change: TextDocumentSyncKind.Incremental, openClose: true }, definitionProvider: { workDoneProgress: true }, declarationProvider: { workDoneProgress: true }, typeDefinitionProvider: { workDoneProgress: true }, @@ -1155,15 +1160,26 @@ export abstract class LanguageServerBase implements LanguageServerInterface, Dis return result; }; - protected onSaveTextDocument = (params: WillSaveTextDocumentParams) => { - const baselineFile = getBaselinedErrors(this.serverOptions.rootDirectory); - const fileKey = this.serverOptions.rootDirectory - .getRelativePath(Uri.file(params.textDocument.uri, this.serviceProvider))! - .toString(); - //cringe - baselineFile.files[fileKey] = diagnosticsToBaseline(this.serverOptions.rootDirectory, [ - this.documentsWithDiagnostics[params.textDocument.uri.toString()], - ]).files[fileKey]; + protected onSaveTextDocument = async (params: WillSaveTextDocumentParams) => { + const fileUri = Uri.file(params.textDocument.uri, this.serviceProvider); + const rootUri = (await this.getWorkspaceForFile(fileUri)).rootUri!; + const baselineFile = getBaselinedErrors(rootUri); + const fileKey = rootUri.getRelativePath(fileUri)!; + const diagnosticsForFile = this.documentsWithDiagnostics[params.textDocument.uri]; + const newDiagnostics = filterOutBaselinedDiagnostics(rootUri, [diagnosticsForFile])[0]; + if (newDiagnostics.diagnostics.length) { + // there are new diagnostics that haven't been baselined, so we don't want to write them + // because the user will have to either fix the diagnostics or explicitly write them to the + // baseline themselves + return; + } + if (diagnosticsForFile) { + //cringe + baselineFile.files[fileKey] = diagnosticsToBaseline(rootUri, [diagnosticsForFile]).files[fileKey]; + } else { + baselineFile.files[fileKey] = []; + } + writeBaselineFile(rootUri, baselineFile); }; protected async onExecuteCommand( @@ -1237,11 +1253,11 @@ export abstract class LanguageServerBase implements LanguageServerInterface, Dis fileDiagnostics: FileDiagnostics ): Promise { const workspace = await this.getWorkspaceForFile(fileDiagnostics.fileUri); - filterOutBaselinedDiagnostics(workspace.rootUri!, [fileDiagnostics]); + const filteredDiagnostics = filterOutBaselinedDiagnostics(workspace.rootUri!, [fileDiagnostics])[0]; return { - uri: convertUriToLspUriString(fs, fileDiagnostics.fileUri), - version: fileDiagnostics.version, - diagnostics: this._convertDiagnostics(fs, fileDiagnostics.diagnostics), + uri: convertUriToLspUriString(fs, filteredDiagnostics.fileUri), + version: filteredDiagnostics.version, + diagnostics: this._convertDiagnostics(fs, filteredDiagnostics.diagnostics), }; } diff --git a/packages/pyright-internal/src/pyright.ts b/packages/pyright-internal/src/pyright.ts index 115f6ae45..3fb3168ca 100644 --- a/packages/pyright-internal/src/pyright.ts +++ b/packages/pyright-internal/src/pyright.ts @@ -48,7 +48,7 @@ import * as core from '@actions/core'; import * as command from '@actions/core/lib/command'; import { convertDiagnostics } from 'pyright-to-gitlab-ci/src/converter'; import path from 'path'; -import { filterOutBaselinedDiagnostics, writeBaseline } from './baseline'; +import { filterOutBaselinedDiagnostics, writeDiagnosticsToBaselineFile } from './baseline'; type SeverityLevel = 'error' | 'warning' | 'information'; @@ -479,9 +479,9 @@ async function runSingleThreaded( ? Uri.file(options.executionRoot ?? '', service.serviceProvider) : options.executionRoot; if (args.writebaseline) { - writeBaseline(rootDir, results.diagnostics); + writeDiagnosticsToBaselineFile(rootDir, results.diagnostics); } - filterOutBaselinedDiagnostics(rootDir, results.diagnostics); + results.diagnostics = filterOutBaselinedDiagnostics(rootDir, results.diagnostics); let errorCount = 0; if (!args.createstub && !args.verifytypes) { let report: DiagnosticResult; From 0bcf17ec6f812c395143b00cacc5326f56759edf Mon Sep 17 00:00:00 2001 From: detachhead Date: Tue, 27 Aug 2024 23:29:47 +1000 Subject: [PATCH 05/28] don't remove baselined errors from unopened files when running the writebaseline task --- packages/pyright-internal/src/baseline.ts | 15 +++++++++++++-- .../src/commands/writeBaseline.ts | 3 +-- packages/pyright-internal/src/pyright.ts | 2 +- 3 files changed, 15 insertions(+), 5 deletions(-) diff --git a/packages/pyright-internal/src/baseline.ts b/packages/pyright-internal/src/baseline.ts index 202fb7e49..4bf972d63 100644 --- a/packages/pyright-internal/src/baseline.ts +++ b/packages/pyright-internal/src/baseline.ts @@ -45,8 +45,19 @@ export const writeBaselineFile = (rootDir: Uri, baselineData: BaselineFile) => { writeFileSync(baselineFile.getPath(), JSON.stringify(baselineData, undefined, 4)); }; -export const writeDiagnosticsToBaselineFile = async (rootDir: Uri, filesWithDiagnostics: FileDiagnostics[]) => { - const baselineData = diagnosticsToBaseline(rootDir, filesWithDiagnostics); +/** + * @param openFilesOnly whether or not the diagnostics were only reported on the open files. setting this to `true` prevents + * it from deleting baselined errors from files that weren't opened + */ +export const writeDiagnosticsToBaselineFile = async ( + rootDir: Uri, + filesWithDiagnostics: FileDiagnostics[], + openFilesOnly: boolean +) => { + let baselineData = diagnosticsToBaseline(rootDir, filesWithDiagnostics); + if (openFilesOnly) { + baselineData = { files: { ...getBaselinedErrors(rootDir).files, ...baselineData.files } }; + } writeBaselineFile(rootDir, baselineData); }; diff --git a/packages/pyright-internal/src/commands/writeBaseline.ts b/packages/pyright-internal/src/commands/writeBaseline.ts index d339fd054..52067fc06 100644 --- a/packages/pyright-internal/src/commands/writeBaseline.ts +++ b/packages/pyright-internal/src/commands/writeBaseline.ts @@ -14,7 +14,6 @@ export class WriteBaselineCommand implements ServerCommand { this._ls.documentsWithDiagnostics[Object.keys(this._ls.documentsWithDiagnostics)[0]].fileUri ) ).rootUri!; - // TODO: this very likely will delete any baselined errors from files that aren't open. FIX BEFORE MERGE!!!!! - return writeDiagnosticsToBaselineFile(workspaceRoot, Object.values(this._ls.documentsWithDiagnostics)); + return writeDiagnosticsToBaselineFile(workspaceRoot, Object.values(this._ls.documentsWithDiagnostics), true); } } diff --git a/packages/pyright-internal/src/pyright.ts b/packages/pyright-internal/src/pyright.ts index 3fb3168ca..a6ee2da94 100644 --- a/packages/pyright-internal/src/pyright.ts +++ b/packages/pyright-internal/src/pyright.ts @@ -479,7 +479,7 @@ async function runSingleThreaded( ? Uri.file(options.executionRoot ?? '', service.serviceProvider) : options.executionRoot; if (args.writebaseline) { - writeDiagnosticsToBaselineFile(rootDir, results.diagnostics); + writeDiagnosticsToBaselineFile(rootDir, results.diagnostics, false); } results.diagnostics = filterOutBaselinedDiagnostics(rootDir, results.diagnostics); let errorCount = 0; From 14d87122ec12749f168a99e4c7a4bda6fe89cf7c Mon Sep 17 00:00:00 2001 From: detachhead Date: Thu, 29 Aug 2024 20:18:04 +1000 Subject: [PATCH 06/28] don't rely on the message for baseline matching because they change often and won't match if a user is running basedpyright with a different language --- packages/pyright-internal/src/baseline.ts | 3 --- 1 file changed, 3 deletions(-) diff --git a/packages/pyright-internal/src/baseline.ts b/packages/pyright-internal/src/baseline.ts index 4bf972d63..890284d4a 100644 --- a/packages/pyright-internal/src/baseline.ts +++ b/packages/pyright-internal/src/baseline.ts @@ -8,7 +8,6 @@ interface BaselineFile { files: { [filePath: string]: { code: DiagnosticRule | undefined; - message: string; range: Range; }[]; }; @@ -31,7 +30,6 @@ export const diagnosticsToBaseline = (rootDir: Uri, filesWithDiagnostics: FileDi baselineData.files[filePath].push( ...fileWithDiagnostics.diagnostics.map((diagnostic) => ({ code: diagnostic.getRule() as DiagnosticRule | undefined, - message: diagnostic.message, range: diagnostic.range, })) ); @@ -88,7 +86,6 @@ export const filterOutBaselinedDiagnostics = ( diagnostics: fileWithDiagnostics.diagnostics.filter((diagnostic) => { const matchedIndex = baselinedErrorsForFile.findIndex( (baselinedError) => - baselinedError.message === diagnostic.message && baselinedError.code === diagnostic.getRule() && baselinedError.range.start.character === diagnostic.range.start.character && baselinedError.range.end.character === diagnostic.range.end.character From 6c0b788b4f22160a1cf47b220650dccb3d830376 Mon Sep 17 00:00:00 2001 From: detachhead Date: Thu, 29 Aug 2024 21:11:15 +1000 Subject: [PATCH 07/28] don't baaseline unreachable/unused/deprecated diagnostic levels as they aren't errors --- packages/pyright-internal/src/baseline.ts | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/packages/pyright-internal/src/baseline.ts b/packages/pyright-internal/src/baseline.ts index 890284d4a..d30d68fba 100644 --- a/packages/pyright-internal/src/baseline.ts +++ b/packages/pyright-internal/src/baseline.ts @@ -3,6 +3,7 @@ import { FileDiagnostics } from './common/diagnosticSink'; import { Range } from './common/textRange'; import { mkdirSync, readFileSync, writeFileSync } from 'fs'; import { Uri } from './common/uri/uri'; +import { DiagnosticCategory } from './common/diagnostic'; interface BaselineFile { files: { @@ -21,14 +22,22 @@ export const diagnosticsToBaseline = (rootDir: Uri, filesWithDiagnostics: FileDi }; for (const fileWithDiagnostics of filesWithDiagnostics) { const filePath = rootDir.getRelativePath(fileWithDiagnostics.fileUri)!.toString(); - if (!fileWithDiagnostics.diagnostics.length) { + const errorDiagnostics = fileWithDiagnostics.diagnostics.filter( + (diagnostic) => + ![ + DiagnosticCategory.Deprecated, + DiagnosticCategory.UnreachableCode, + DiagnosticCategory.UnusedCode, + ].includes(diagnostic.category) + ); + if (!errorDiagnostics.length) { continue; } if (!(filePath in baselineData.files)) { baselineData.files[filePath] = []; } baselineData.files[filePath].push( - ...fileWithDiagnostics.diagnostics.map((diagnostic) => ({ + ...errorDiagnostics.map((diagnostic) => ({ code: diagnostic.getRule() as DiagnosticRule | undefined, range: diagnostic.range, })) From 366ebca2c5468ad1c93af343732b07fbedf580a9 Mon Sep 17 00:00:00 2001 From: detachhead Date: Thu, 29 Aug 2024 23:05:57 +1000 Subject: [PATCH 08/28] report baselined errors that can be represented with a hint diagnostic (eg. unused, deprecated) as such --- packages/pyright-internal/src/baseline.ts | 61 +++++++++++++------ .../pyright-internal/src/common/diagnostic.ts | 14 ++++- .../src/languageServerBase.ts | 3 +- 3 files changed, 55 insertions(+), 23 deletions(-) diff --git a/packages/pyright-internal/src/baseline.ts b/packages/pyright-internal/src/baseline.ts index d30d68fba..07be5013d 100644 --- a/packages/pyright-internal/src/baseline.ts +++ b/packages/pyright-internal/src/baseline.ts @@ -3,7 +3,8 @@ import { FileDiagnostics } from './common/diagnosticSink'; import { Range } from './common/textRange'; import { mkdirSync, readFileSync, writeFileSync } from 'fs'; import { Uri } from './common/uri/uri'; -import { DiagnosticCategory } from './common/diagnostic'; +import { convertLevelToCategory, Diagnostic, DiagnosticCategory } from './common/diagnostic'; +import { extraOptionDiagnosticRules } from './common/configOptions'; interface BaselineFile { files: { @@ -79,33 +80,55 @@ export const getBaselinedErrors = (rootDir: Uri): BaselineFile => { return JSON.parse(baselineFileContents); }; +interface FileDiagnosticsWithBaselineInfo extends FileDiagnostics { + containsNewErrors: boolean; +} + export const filterOutBaselinedDiagnostics = ( rootDir: Uri, filesWithDiagnostics: readonly FileDiagnostics[] -): FileDiagnostics[] => { +): FileDiagnosticsWithBaselineInfo[] => { const baselineFile = getBaselinedErrors(rootDir); return filesWithDiagnostics.map((fileWithDiagnostics) => { const baselinedErrorsForFile = baselineFile.files[rootDir.getRelativePath(fileWithDiagnostics.fileUri)!.toString()]; if (!baselinedErrorsForFile) { - return fileWithDiagnostics; + return { ...fileWithDiagnostics, containsNewErrors: true }; } - return { - ...fileWithDiagnostics, - diagnostics: fileWithDiagnostics.diagnostics.filter((diagnostic) => { - const matchedIndex = baselinedErrorsForFile.findIndex( - (baselinedError) => - baselinedError.code === diagnostic.getRule() && - baselinedError.range.start.character === diagnostic.range.start.character && - baselinedError.range.end.character === diagnostic.range.end.character - ); - if (matchedIndex >= 0) { - baselinedErrorsForFile.splice(matchedIndex, 1); - return false; - } else { - return true; + const filteredDiagnostics = []; + let containsNewErrors = false; + for (let diagnostic of fileWithDiagnostics.diagnostics) { + const diagnosticRule = diagnostic.getRule() as DiagnosticRule | undefined; + const matchedIndex = baselinedErrorsForFile.findIndex( + (baselinedError) => + baselinedError.code === diagnosticRule && + baselinedError.range.start.character === diagnostic.range.start.character && + baselinedError.range.end.character === diagnostic.range.end.character + ); + if (matchedIndex >= 0) { + baselinedErrorsForFile.splice(matchedIndex, 1); + // if the baselined error can be reported as a hint (eg. unreachable/deprecated), keep it and change its diagnostic level to that instead + // TODO: should we only baseline errors and not warnings/notes? + if (diagnosticRule) { + for (const { name, get } of extraOptionDiagnosticRules) { + if (get().includes(diagnosticRule)) { + diagnostic = new Diagnostic( + convertLevelToCategory(name), + diagnostic.message, + diagnostic.range, + diagnostic.priority + ); + filteredDiagnostics.push(diagnostic); + // none of these rules should have multiple extra diagnostic levels so we break after the first match + break; + } + } } - }), - }; + } else { + containsNewErrors = true; + filteredDiagnostics.push(diagnostic); + } + } + return { ...fileWithDiagnostics, diagnostics: filteredDiagnostics, containsNewErrors }; }); }; diff --git a/packages/pyright-internal/src/common/diagnostic.ts b/packages/pyright-internal/src/common/diagnostic.ts index a9ae6ea69..bc0c3cf36 100644 --- a/packages/pyright-internal/src/common/diagnostic.ts +++ b/packages/pyright-internal/src/common/diagnostic.ts @@ -9,7 +9,7 @@ import { Commands } from '../commands/commands'; import { appendArray } from './collectionUtils'; -import { DiagnosticLevel } from './configOptions'; +import { LspDiagnosticLevel } from './configOptions'; import { Range, TextRange } from './textRange'; import { Uri } from './uri/uri'; @@ -43,7 +43,7 @@ export const enum DiagnosticCategory { TaskItem, } -export function convertLevelToCategory(level: DiagnosticLevel) { +export function convertLevelToCategory(level: LspDiagnosticLevel) { switch (level) { case 'error': return DiagnosticCategory.Error; @@ -54,7 +54,17 @@ export function convertLevelToCategory(level: DiagnosticLevel) { case 'information': return DiagnosticCategory.Information; + case 'unreachable': + return DiagnosticCategory.UnreachableCode; + + case 'unused': + return DiagnosticCategory.UnusedCode; + + case 'deprecated': + return DiagnosticCategory.Deprecated; + default: + level satisfies 'none'; throw new Error(`${level} is not expected`); } } diff --git a/packages/pyright-internal/src/languageServerBase.ts b/packages/pyright-internal/src/languageServerBase.ts index ca4fc5b86..2b81b72a5 100644 --- a/packages/pyright-internal/src/languageServerBase.ts +++ b/packages/pyright-internal/src/languageServerBase.ts @@ -1166,8 +1166,7 @@ export abstract class LanguageServerBase implements LanguageServerInterface, Dis const baselineFile = getBaselinedErrors(rootUri); const fileKey = rootUri.getRelativePath(fileUri)!; const diagnosticsForFile = this.documentsWithDiagnostics[params.textDocument.uri]; - const newDiagnostics = filterOutBaselinedDiagnostics(rootUri, [diagnosticsForFile])[0]; - if (newDiagnostics.diagnostics.length) { + if (diagnosticsForFile && filterOutBaselinedDiagnostics(rootUri, [diagnosticsForFile])[0].containsNewErrors) { // there are new diagnostics that haven't been baselined, so we don't want to write them // because the user will have to either fix the diagnostics or explicitly write them to the // baseline themselves From 2e9332a1d81f36b5d3878172112b7d39d034ab02 Mon Sep 17 00:00:00 2001 From: detachhead Date: Mon, 2 Sep 2024 19:47:52 +1000 Subject: [PATCH 09/28] output baseline info in the cli --- packages/pyright-internal/src/baseline.ts | 23 ++++++---- .../src/languageServerBase.ts | 6 ++- packages/pyright-internal/src/pyright.ts | 44 +++++++++++++++---- 3 files changed, 55 insertions(+), 18 deletions(-) diff --git a/packages/pyright-internal/src/baseline.ts b/packages/pyright-internal/src/baseline.ts index 07be5013d..f82b6b66c 100644 --- a/packages/pyright-internal/src/baseline.ts +++ b/packages/pyright-internal/src/baseline.ts @@ -6,12 +6,14 @@ import { Uri } from './common/uri/uri'; import { convertLevelToCategory, Diagnostic, DiagnosticCategory } from './common/diagnostic'; import { extraOptionDiagnosticRules } from './common/configOptions'; +interface BaselinedDiagnostic { + code: DiagnosticRule | undefined; + range: Range; +} + interface BaselineFile { files: { - [filePath: string]: { - code: DiagnosticRule | undefined; - range: Range; - }[]; + [filePath: string]: BaselinedDiagnostic[]; }; } @@ -81,7 +83,7 @@ export const getBaselinedErrors = (rootDir: Uri): BaselineFile => { }; interface FileDiagnosticsWithBaselineInfo extends FileDiagnostics { - containsNewErrors: boolean; + alreadyBaselinedDiagnostics: BaselinedDiagnostic[]; } export const filterOutBaselinedDiagnostics = ( @@ -93,10 +95,10 @@ export const filterOutBaselinedDiagnostics = ( const baselinedErrorsForFile = baselineFile.files[rootDir.getRelativePath(fileWithDiagnostics.fileUri)!.toString()]; if (!baselinedErrorsForFile) { - return { ...fileWithDiagnostics, containsNewErrors: true }; + return { ...fileWithDiagnostics, alreadyBaselinedDiagnostics: [] }; } + const originalBaselinedErrorsForFile = [...baselinedErrorsForFile]; const filteredDiagnostics = []; - let containsNewErrors = false; for (let diagnostic of fileWithDiagnostics.diagnostics) { const diagnosticRule = diagnostic.getRule() as DiagnosticRule | undefined; const matchedIndex = baselinedErrorsForFile.findIndex( @@ -125,10 +127,13 @@ export const filterOutBaselinedDiagnostics = ( } } } else { - containsNewErrors = true; filteredDiagnostics.push(diagnostic); } } - return { ...fileWithDiagnostics, diagnostics: filteredDiagnostics, containsNewErrors }; + return { + ...fileWithDiagnostics, + diagnostics: filteredDiagnostics, + alreadyBaselinedDiagnostics: originalBaselinedErrorsForFile, + }; }); }; diff --git a/packages/pyright-internal/src/languageServerBase.ts b/packages/pyright-internal/src/languageServerBase.ts index 2b81b72a5..9071b9ab9 100644 --- a/packages/pyright-internal/src/languageServerBase.ts +++ b/packages/pyright-internal/src/languageServerBase.ts @@ -1166,7 +1166,11 @@ export abstract class LanguageServerBase implements LanguageServerInterface, Dis const baselineFile = getBaselinedErrors(rootUri); const fileKey = rootUri.getRelativePath(fileUri)!; const diagnosticsForFile = this.documentsWithDiagnostics[params.textDocument.uri]; - if (diagnosticsForFile && filterOutBaselinedDiagnostics(rootUri, [diagnosticsForFile])[0].containsNewErrors) { + if ( + diagnosticsForFile && + diagnosticsForFile.diagnostics.length > + filterOutBaselinedDiagnostics(rootUri, [diagnosticsForFile])[0].alreadyBaselinedDiagnostics.length + ) { // there are new diagnostics that haven't been baselined, so we don't want to write them // because the user will have to either fix the diagnostics or explicitly write them to the // baseline themselves diff --git a/packages/pyright-internal/src/pyright.ts b/packages/pyright-internal/src/pyright.ts index a6ee2da94..6e96f1df1 100644 --- a/packages/pyright-internal/src/pyright.ts +++ b/packages/pyright-internal/src/pyright.ts @@ -48,7 +48,7 @@ import * as core from '@actions/core'; import * as command from '@actions/core/lib/command'; import { convertDiagnostics } from 'pyright-to-gitlab-ci/src/converter'; import path from 'path'; -import { filterOutBaselinedDiagnostics, writeDiagnosticsToBaselineFile } from './baseline'; +import { baselineFilePath, filterOutBaselinedDiagnostics, writeDiagnosticsToBaselineFile } from './baseline'; type SeverityLevel = 'error' | 'warning' | 'information'; @@ -478,10 +478,36 @@ async function runSingleThreaded( typeof options.executionRoot === 'string' || options.executionRoot === undefined ? Uri.file(options.executionRoot ?? '', service.serviceProvider) : options.executionRoot; - if (args.writebaseline) { - writeDiagnosticsToBaselineFile(rootDir, results.diagnostics, false); + const allDiagnostics = results.diagnostics; + const filteredDiagnostics = filterOutBaselinedDiagnostics(rootDir, results.diagnostics); + if (args.writebaseline || !filteredDiagnostics.length) { + writeDiagnosticsToBaselineFile(rootDir, allDiagnostics, false); + const previousErrorCount = filteredDiagnostics + .flatMap((file) => file.alreadyBaselinedDiagnostics.length) + .reduce((prev, next) => prev + next); + const newErrorCount = allDiagnostics + .flatMap((file) => file.diagnostics.length) + .reduce((prev, next) => prev + next); + const diff = newErrorCount - previousErrorCount; + let message = ''; + if (diff === 0) { + message += "error count didn't change"; + } else if (diff > 0) { + message += `went up by ${diff}`; + } else { + message += `went down by ${diff * -1}`; + } + const totalErrorCount = allDiagnostics + .map((file) => file.diagnostics.length) + .reduce((prev, count) => prev + count); + console.info( + `updated ${rootDir.getRelativePath(baselineFilePath(rootDir))} with ${pluralize( + totalErrorCount, + 'error' + )} (${message})` + ); } - results.diagnostics = filterOutBaselinedDiagnostics(rootDir, results.diagnostics); + results.diagnostics = filteredDiagnostics; // TODO: is this needed? let errorCount = 0; if (!args.createstub && !args.verifytypes) { let report: DiagnosticResult; @@ -1266,15 +1292,17 @@ function convertDiagnosticToJson(filePath: string, diag: Diagnostic): PyrightJso rule: diag.getRule(), }; } -const pluralize = (n: number, singular: string, plural: string) => `${n} ${n === 1 ? singular : plural}`; + +const pluralize = (n: number, singular: string, plural: string = `${singular}s`) => + `${n} ${n === 1 ? singular : plural}`; const printDiagnosticSummary = (result: DiagnosticResult) => { console.info( [ - pluralize(result.errorCount, 'error', 'errors'), - pluralize(result.warningCount, 'warning', 'warnings'), + pluralize(result.errorCount, 'error'), + pluralize(result.warningCount, 'warning'), // we use the word "notes" instead because "informations" sounds dumb - pluralize(result.informationCount, 'note', 'notes'), + pluralize(result.informationCount, 'note'), ].join(', ') ); }; From 33f7cb5d0e4e9b1ad03ab6eee12620c16aeeda38 Mon Sep 17 00:00:00 2001 From: detachhead Date: Tue, 3 Sep 2024 23:47:48 +1000 Subject: [PATCH 10/28] fix incorrect baseline error counts --- packages/pyright-internal/src/pyright.ts | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/packages/pyright-internal/src/pyright.ts b/packages/pyright-internal/src/pyright.ts index 6e96f1df1..18b89aa7a 100644 --- a/packages/pyright-internal/src/pyright.ts +++ b/packages/pyright-internal/src/pyright.ts @@ -480,14 +480,21 @@ async function runSingleThreaded( : options.executionRoot; const allDiagnostics = results.diagnostics; const filteredDiagnostics = filterOutBaselinedDiagnostics(rootDir, results.diagnostics); - if (args.writebaseline || !filteredDiagnostics.length) { + const newErrorCount = allDiagnostics + .flatMap( + (file) => + file.diagnostics.filter((diagnostic) => + [DiagnosticCategory.Error, DiagnosticCategory.Warning, DiagnosticCategory.Information].includes( + diagnostic.category + ) + ).length + ) + .reduce((prev, next) => prev + next); + if (args.writebaseline || !newErrorCount) { writeDiagnosticsToBaselineFile(rootDir, allDiagnostics, false); const previousErrorCount = filteredDiagnostics .flatMap((file) => file.alreadyBaselinedDiagnostics.length) .reduce((prev, next) => prev + next); - const newErrorCount = allDiagnostics - .flatMap((file) => file.diagnostics.length) - .reduce((prev, next) => prev + next); const diff = newErrorCount - previousErrorCount; let message = ''; if (diff === 0) { @@ -497,12 +504,9 @@ async function runSingleThreaded( } else { message += `went down by ${diff * -1}`; } - const totalErrorCount = allDiagnostics - .map((file) => file.diagnostics.length) - .reduce((prev, count) => prev + count); console.info( `updated ${rootDir.getRelativePath(baselineFilePath(rootDir))} with ${pluralize( - totalErrorCount, + newErrorCount, 'error' )} (${message})` ); From 7a5ab46302fca1f8f2a2d6dd19bc65b615823bc0 Mon Sep 17 00:00:00 2001 From: detachhead Date: Tue, 3 Sep 2024 23:48:09 +1000 Subject: [PATCH 11/28] fix python tests not being typechecked --- pyproject.toml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 7ad60642d..e0f22583d 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -192,10 +192,10 @@ max-line-length = 200 [tool.basedpyright] pythonVersion = "3.8" -include = ["basedpyright", "based_build", "pdm_build.py"] +include = ["basedpyright", "based_build", "pdm_build.py", "tests"] ignore = ["pw", "basedpyright/dist", "packages", "build"] typeCheckingMode = "all" -reportImplicitStringConcatenation = false # conflicts with ruff formatter +reportImplicitStringConcatenation = false # conflicts with ruff formatter # currently commented out due to https://github.com/DetachHead/basedpyright/issues/570 # only seem to be needed when using pycharm, which i dont really care about anyway because pycharm sucks From 3443af9c54338f378c1e4b8aa892dcbc9050e87c Mon Sep 17 00:00:00 2001 From: detachhead Date: Wed, 11 Sep 2024 21:56:04 +1000 Subject: [PATCH 12/28] fix crash when reading/writing baseline when no workspace is open and reanalyze the file after running the writeBaseline task --- .../src/analyzer/backgroundAnalysisProgram.ts | 3 ++- .../pyright-internal/src/analyzer/service.ts | 5 ++++ packages/pyright-internal/src/baseline.ts | 11 +++++---- .../src/commands/writeBaseline.ts | 21 +++++++++++----- .../src/languageServerBase.ts | 24 +++++++++++-------- packages/pyright-internal/src/pyright.ts | 4 ++-- 6 files changed, 45 insertions(+), 23 deletions(-) diff --git a/packages/pyright-internal/src/analyzer/backgroundAnalysisProgram.ts b/packages/pyright-internal/src/analyzer/backgroundAnalysisProgram.ts index d5abe8dad..fd812036b 100644 --- a/packages/pyright-internal/src/analyzer/backgroundAnalysisProgram.ts +++ b/packages/pyright-internal/src/analyzer/backgroundAnalysisProgram.ts @@ -26,7 +26,8 @@ export enum InvalidatedReason { SourceWatcherChanged, LibraryWatcherChanged, LibraryWatcherContentOnlyChanged, - Nunya, // TODO: whats the actual reason + BaselineFileUpdated, + Nunya, // TODO: whats the actual reason? this is used by the browser package } export class BackgroundAnalysisProgram { diff --git a/packages/pyright-internal/src/analyzer/service.ts b/packages/pyright-internal/src/analyzer/service.ts index c22babfa5..ea51fb653 100644 --- a/packages/pyright-internal/src/analyzer/service.ts +++ b/packages/pyright-internal/src/analyzer/service.ts @@ -479,6 +479,11 @@ export class AnalyzerService { this._backgroundAnalysisProgram.restart(); } + baselineUpdated = () => { + this.invalidateAndForceReanalysis(InvalidatedReason.BaselineFileUpdated); + this._scheduleReanalysis(false); + }; + protected runAnalysis(token: CancellationToken) { // This creates a cancellation source only if it actually gets used. const moreToAnalyze = this._backgroundAnalysisProgram.startAnalysis(token); diff --git a/packages/pyright-internal/src/baseline.ts b/packages/pyright-internal/src/baseline.ts index f82b6b66c..501946c5a 100644 --- a/packages/pyright-internal/src/baseline.ts +++ b/packages/pyright-internal/src/baseline.ts @@ -83,19 +83,22 @@ export const getBaselinedErrors = (rootDir: Uri): BaselineFile => { }; interface FileDiagnosticsWithBaselineInfo extends FileDiagnostics { - alreadyBaselinedDiagnostics: BaselinedDiagnostic[]; + alreadyBaselinedDiagnostics?: BaselinedDiagnostic[]; } export const filterOutBaselinedDiagnostics = ( - rootDir: Uri, + rootDir: Uri | undefined, filesWithDiagnostics: readonly FileDiagnostics[] -): FileDiagnosticsWithBaselineInfo[] => { +): readonly FileDiagnosticsWithBaselineInfo[] => { + if (!rootDir) { + return filesWithDiagnostics; + } const baselineFile = getBaselinedErrors(rootDir); return filesWithDiagnostics.map((fileWithDiagnostics) => { const baselinedErrorsForFile = baselineFile.files[rootDir.getRelativePath(fileWithDiagnostics.fileUri)!.toString()]; if (!baselinedErrorsForFile) { - return { ...fileWithDiagnostics, alreadyBaselinedDiagnostics: [] }; + return fileWithDiagnostics; } const originalBaselinedErrorsForFile = [...baselinedErrorsForFile]; const filteredDiagnostics = []; diff --git a/packages/pyright-internal/src/commands/writeBaseline.ts b/packages/pyright-internal/src/commands/writeBaseline.ts index 52067fc06..ffd5f2a9b 100644 --- a/packages/pyright-internal/src/commands/writeBaseline.ts +++ b/packages/pyright-internal/src/commands/writeBaseline.ts @@ -9,11 +9,20 @@ export class WriteBaselineCommand implements ServerCommand { async execute(): Promise { // TODO: figure out a better way to get workspace root - const workspaceRoot = ( - await this._ls.getWorkspaceForFile( - this._ls.documentsWithDiagnostics[Object.keys(this._ls.documentsWithDiagnostics)[0]].fileUri - ) - ).rootUri!; - return writeDiagnosticsToBaselineFile(workspaceRoot, Object.values(this._ls.documentsWithDiagnostics), true); + const firstFile = this._ls.documentsWithDiagnostics[Object.keys(this._ls.documentsWithDiagnostics)[0]]?.fileUri; + if (firstFile) { + const workspace = await this._ls.getWorkspaceForFile(firstFile); + const workspaceRoot = workspace.rootUri; + if (workspaceRoot) { + await writeDiagnosticsToBaselineFile( + workspaceRoot, + Object.values(this._ls.documentsWithDiagnostics), + true + ); + workspace.service.baselineUpdated(); + return; + } + } + this._ls.window.showErrorMessage('cannot write to the baseline file because no workspace is open'); } } diff --git a/packages/pyright-internal/src/languageServerBase.ts b/packages/pyright-internal/src/languageServerBase.ts index 9071b9ab9..4be6bf472 100644 --- a/packages/pyright-internal/src/languageServerBase.ts +++ b/packages/pyright-internal/src/languageServerBase.ts @@ -1162,19 +1162,23 @@ export abstract class LanguageServerBase implements LanguageServerInterface, Dis protected onSaveTextDocument = async (params: WillSaveTextDocumentParams) => { const fileUri = Uri.file(params.textDocument.uri, this.serviceProvider); - const rootUri = (await this.getWorkspaceForFile(fileUri)).rootUri!; + const workspace = await this.getWorkspaceForFile(fileUri); + const rootUri = workspace.rootUri!; const baselineFile = getBaselinedErrors(rootUri); const fileKey = rootUri.getRelativePath(fileUri)!; const diagnosticsForFile = this.documentsWithDiagnostics[params.textDocument.uri]; - if ( - diagnosticsForFile && - diagnosticsForFile.diagnostics.length > - filterOutBaselinedDiagnostics(rootUri, [diagnosticsForFile])[0].alreadyBaselinedDiagnostics.length - ) { - // there are new diagnostics that haven't been baselined, so we don't want to write them - // because the user will have to either fix the diagnostics or explicitly write them to the - // baseline themselves - return; + if (diagnosticsForFile) { + const filteredDiagnostics = filterOutBaselinedDiagnostics(rootUri, [diagnosticsForFile])[0]; + if ( + // no baseline file exists + !filteredDiagnostics.alreadyBaselinedDiagnostics || + // there are new diagnostics that haven't been baselined, so we don't want to write them + // because the user will have to either fix the diagnostics or explicitly write them to the + // baseline themselves + diagnosticsForFile.diagnostics.length > filteredDiagnostics.alreadyBaselinedDiagnostics.length + ) { + return; + } } if (diagnosticsForFile) { //cringe diff --git a/packages/pyright-internal/src/pyright.ts b/packages/pyright-internal/src/pyright.ts index 18b89aa7a..233ef3c6d 100644 --- a/packages/pyright-internal/src/pyright.ts +++ b/packages/pyright-internal/src/pyright.ts @@ -493,7 +493,7 @@ async function runSingleThreaded( if (args.writebaseline || !newErrorCount) { writeDiagnosticsToBaselineFile(rootDir, allDiagnostics, false); const previousErrorCount = filteredDiagnostics - .flatMap((file) => file.alreadyBaselinedDiagnostics.length) + .flatMap((file) => file.alreadyBaselinedDiagnostics?.length ?? 0) .reduce((prev, next) => prev + next); const diff = newErrorCount - previousErrorCount; let message = ''; @@ -511,7 +511,7 @@ async function runSingleThreaded( )} (${message})` ); } - results.diagnostics = filteredDiagnostics; // TODO: is this needed? + results.diagnostics = [...filteredDiagnostics]; // TODO: is this needed? let errorCount = 0; if (!args.createstub && !args.verifytypes) { let report: DiagnosticResult; From 9d223ae03eeb35a87f0ff886fd8fe4ac11668609 Mon Sep 17 00:00:00 2001 From: detachhead Date: Sun, 15 Sep 2024 12:04:21 +1000 Subject: [PATCH 13/28] fix vscode jest --- .vscode/settings.json | 3 ++- package.json | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/.vscode/settings.json b/.vscode/settings.json index 5da277e3a..b018313b5 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -76,5 +76,6 @@ "task.problemMatchers.neverPrompt": true, "editor.unicodeHighlight.nonBasicASCII": true, "ruff.nativeServer": true, - "typescript.inlayHints.parameterNames.enabled": "literals" + "typescript.inlayHints.parameterNames.enabled": "literals", + "jest.jestCommandLine": "npm run jest --" } diff --git a/package.json b/package.json index fbc93ecf2..84e078e9f 100644 --- a/package.json +++ b/package.json @@ -21,7 +21,8 @@ "fix:prettier": "prettier --write .", "typecheck": "npx lerna exec --stream --no-bail --ignore=pyright -- tsc --noEmit", "run:cli:windows": "npm run build:cli:dev && node packages/pyright/index.js --pythonpath .venv/Scripts/python.exe", - "run:cli:unix": "npm run build:cli:dev && node packages/pyright/index.js --pythonpath .venv/bin/python" + "run:cli:unix": "npm run build:cli:dev && node packages/pyright/index.js --pythonpath .venv/bin/python", + "jest": "cd packages/pyright-internal && jest" }, "devDependencies": { "@detachhead/ts-helpers": "^16.2.0", From 82e0b92eb0f67d8c470bd25374e7df88a4b77988 Mon Sep 17 00:00:00 2001 From: detachhead Date: Sun, 15 Sep 2024 23:30:41 +1000 Subject: [PATCH 14/28] refactor baseline stuff to make it easier to write tests for --- .../pyright-internal/src/analyzer/analysis.ts | 4 +- .../pyright-internal/src/analyzer/program.ts | 2 +- .../src/analyzer/sourceFile.ts | 3 + packages/pyright-internal/src/baseline.ts | 113 +++++++-------- .../pyright-internal/src/common/diagnostic.ts | 1 + .../src/languageServerBase.ts | 136 +++++++++--------- packages/pyright-internal/src/pyright.ts | 50 +++---- .../src/tests/baseline.test.ts | 13 ++ .../.basedpyright/baseline.json | 19 +++ .../baselined_error_not_reported/foo.py | 2 + 10 files changed, 188 insertions(+), 155 deletions(-) create mode 100644 packages/pyright-internal/src/tests/baseline.test.ts create mode 100644 packages/pyright-internal/src/tests/samples/baseline/baselined_error_not_reported/.basedpyright/baseline.json create mode 100644 packages/pyright-internal/src/tests/samples/baseline/baselined_error_not_reported/foo.py diff --git a/packages/pyright-internal/src/analyzer/analysis.ts b/packages/pyright-internal/src/analyzer/analysis.ts index aef7d6ec1..c551586e5 100644 --- a/packages/pyright-internal/src/analyzer/analysis.ts +++ b/packages/pyright-internal/src/analyzer/analysis.ts @@ -12,16 +12,16 @@ import { OperationCanceledException, throwIfCancellationRequested } from '../com import { ConfigOptions } from '../common/configOptions'; import { ConsoleInterface } from '../common/console'; import * as debug from '../common/debug'; -import { FileDiagnostics } from '../common/diagnosticSink'; import { Duration } from '../common/timing'; import { MaxAnalysisTime, Program } from './program'; +import { FileDiagnostics } from '../common/diagnosticSink'; export const nullCallback: AnalysisCompleteCallback = () => { /* empty */ }; export interface AnalysisResults { - diagnostics: FileDiagnostics[]; + diagnostics: readonly FileDiagnostics[]; filesInProgram: number; checkingOnlyOpenFiles: boolean; requiringAnalysisCount: RequiringAnalysisCount; diff --git a/packages/pyright-internal/src/analyzer/program.ts b/packages/pyright-internal/src/analyzer/program.ts index 0e3b1bd05..4af281fa2 100644 --- a/packages/pyright-internal/src/analyzer/program.ts +++ b/packages/pyright-internal/src/analyzer/program.ts @@ -881,7 +881,7 @@ export class Program { }); } - getDiagnostics(options: ConfigOptions, reportDeltasOnly = true): FileDiagnostics[] { + getDiagnostics(options: ConfigOptions, reportDeltasOnly = true): readonly FileDiagnostics[] { const fileDiagnostics: FileDiagnostics[] = this._removeUnneededFiles(); this._sourceFileList.forEach((sourceFileInfo) => { diff --git a/packages/pyright-internal/src/analyzer/sourceFile.ts b/packages/pyright-internal/src/analyzer/sourceFile.ts index 49ad324f5..8c2dcb543 100644 --- a/packages/pyright-internal/src/analyzer/sourceFile.ts +++ b/packages/pyright-internal/src/analyzer/sourceFile.ts @@ -48,6 +48,7 @@ import { SourceMapper } from './sourceMapper'; import { SymbolTable } from './symbol'; import { TestWalker } from './testWalker'; import { TypeEvaluator } from './typeEvaluatorTypes'; +import { filterOutBaselinedDiagnostics } from '../baseline'; // Limit the number of import cycles tracked per source file. const _maxImportCyclesPerFile = 4; @@ -1246,6 +1247,8 @@ export class SourceFile { // Now add in the "unnecessary type ignore" diagnostics. diagList = diagList.concat(unnecessaryTypeIgnoreDiags); + filterOutBaselinedDiagnostics(configOptions.projectRoot, this._uri, diagList); + // If we're not returning any diagnostics, filter out all of // the errors and warnings, leaving only the unreachable code // and deprecated diagnostics. diff --git a/packages/pyright-internal/src/baseline.ts b/packages/pyright-internal/src/baseline.ts index 501946c5a..7fb31de41 100644 --- a/packages/pyright-internal/src/baseline.ts +++ b/packages/pyright-internal/src/baseline.ts @@ -19,7 +19,7 @@ interface BaselineFile { export const baselineFilePath = (rootDir: Uri) => rootDir.combinePaths('.basedpyright/baseline.json'); -export const diagnosticsToBaseline = (rootDir: Uri, filesWithDiagnostics: FileDiagnostics[]): BaselineFile => { +const diagnosticsToBaseline = (rootDir: Uri, filesWithDiagnostics: readonly FileDiagnostics[]): BaselineFile => { const baselineData: BaselineFile = { files: {}, }; @@ -31,14 +31,14 @@ export const diagnosticsToBaseline = (rootDir: Uri, filesWithDiagnostics: FileDi DiagnosticCategory.Deprecated, DiagnosticCategory.UnreachableCode, DiagnosticCategory.UnusedCode, - ].includes(diagnostic.category) + ].includes(diagnostic.category) || diagnostic.baselineStatus === 'baselined with hint' ); - if (!errorDiagnostics.length) { - continue; - } if (!(filePath in baselineData.files)) { baselineData.files[filePath] = []; } + if (!errorDiagnostics.length) { + continue; + } baselineData.files[filePath].push( ...errorDiagnostics.map((diagnostic) => ({ code: diagnostic.getRule() as DiagnosticRule | undefined, @@ -49,7 +49,7 @@ export const diagnosticsToBaseline = (rootDir: Uri, filesWithDiagnostics: FileDi return baselineData; }; -export const writeBaselineFile = (rootDir: Uri, baselineData: BaselineFile) => { +const writeBaselineFile = (rootDir: Uri, baselineData: BaselineFile) => { const baselineFile = baselineFilePath(rootDir); mkdirSync(baselineFile.getDirectory().getPath(), { recursive: true }); writeFileSync(baselineFile.getPath(), JSON.stringify(baselineData, undefined, 4)); @@ -61,13 +61,19 @@ export const writeBaselineFile = (rootDir: Uri, baselineData: BaselineFile) => { */ export const writeDiagnosticsToBaselineFile = async ( rootDir: Uri, - filesWithDiagnostics: FileDiagnostics[], + filesWithDiagnostics: readonly FileDiagnostics[], openFilesOnly: boolean ) => { let baselineData = diagnosticsToBaseline(rootDir, filesWithDiagnostics); if (openFilesOnly) { baselineData = { files: { ...getBaselinedErrors(rootDir).files, ...baselineData.files } }; } + // remove files where there are no errors + for (const file in baselineData.files) { + if (!baselineData.files[file].length) { + delete baselineData.files[file]; + } + } writeBaselineFile(rootDir, baselineData); }; @@ -82,61 +88,52 @@ export const getBaselinedErrors = (rootDir: Uri): BaselineFile => { return JSON.parse(baselineFileContents); }; -interface FileDiagnosticsWithBaselineInfo extends FileDiagnostics { - alreadyBaselinedDiagnostics?: BaselinedDiagnostic[]; -} - -export const filterOutBaselinedDiagnostics = ( - rootDir: Uri | undefined, - filesWithDiagnostics: readonly FileDiagnostics[] -): readonly FileDiagnosticsWithBaselineInfo[] => { - if (!rootDir) { - return filesWithDiagnostics; +export const getBaselinedErrorsForFile = (rootDir: Uri, file: Uri): BaselinedDiagnostic[] => { + const relativePath = rootDir.getRelativePath(file); + let result; + // if this is undefined it means the file isn't in the workspace + if (relativePath) { + result = getBaselinedErrors(rootDir).files[rootDir.getRelativePath(file)!.toString()]; } - const baselineFile = getBaselinedErrors(rootDir); - return filesWithDiagnostics.map((fileWithDiagnostics) => { - const baselinedErrorsForFile = - baselineFile.files[rootDir.getRelativePath(fileWithDiagnostics.fileUri)!.toString()]; - if (!baselinedErrorsForFile) { - return fileWithDiagnostics; - } - const originalBaselinedErrorsForFile = [...baselinedErrorsForFile]; - const filteredDiagnostics = []; - for (let diagnostic of fileWithDiagnostics.diagnostics) { - const diagnosticRule = diagnostic.getRule() as DiagnosticRule | undefined; - const matchedIndex = baselinedErrorsForFile.findIndex( - (baselinedError) => - baselinedError.code === diagnosticRule && - baselinedError.range.start.character === diagnostic.range.start.character && - baselinedError.range.end.character === diagnostic.range.end.character - ); - if (matchedIndex >= 0) { - baselinedErrorsForFile.splice(matchedIndex, 1); - // if the baselined error can be reported as a hint (eg. unreachable/deprecated), keep it and change its diagnostic level to that instead - // TODO: should we only baseline errors and not warnings/notes? - if (diagnosticRule) { - for (const { name, get } of extraOptionDiagnosticRules) { - if (get().includes(diagnosticRule)) { - diagnostic = new Diagnostic( - convertLevelToCategory(name), - diagnostic.message, - diagnostic.range, - diagnostic.priority - ); - filteredDiagnostics.push(diagnostic); - // none of these rules should have multiple extra diagnostic levels so we break after the first match - break; + return result ?? []; +}; + +export const filterOutBaselinedDiagnostics = (rootDir: Uri, file: Uri, diagnostics: Diagnostic[]) => { + const baselinedErrorsForFile = getBaselinedErrorsForFile(rootDir, file); + for (const index in diagnostics) { + const diagnostic = diagnostics[index]; + const diagnosticRule = diagnostic.getRule() as DiagnosticRule | undefined; + const matchedIndex = baselinedErrorsForFile.findIndex( + (baselinedError) => + baselinedError.code === diagnosticRule && + baselinedError.range.start.character === diagnostic.range.start.character && + baselinedError.range.end.character === diagnostic.range.end.character + ); + if (matchedIndex >= 0) { + baselinedErrorsForFile.splice(matchedIndex, 1); + // if the baselined error can be reported as a hint (eg. unreachable/deprecated), keep it and change its diagnostic level to that instead + // TODO: should we only baseline errors and not warnings/notes? + if (diagnosticRule) { + for (const { name, get } of extraOptionDiagnosticRules) { + if (get().includes(diagnosticRule)) { + const newDiagnostic = new Diagnostic( + convertLevelToCategory(name), + diagnostic.message, + diagnostic.range, + diagnostic.priority + ); + newDiagnostic.baselineStatus = 'baselined with hint'; + const rule = diagnostic.getRule(); + if (rule) { + newDiagnostic.setRule(rule); } + diagnostics[index] = newDiagnostic; + // none of these rules should have multiple extra diagnostic levels so we break after the first match + break; } } - } else { - filteredDiagnostics.push(diagnostic); } + diagnostic.baselineStatus = 'baselined'; } - return { - ...fileWithDiagnostics, - diagnostics: filteredDiagnostics, - alreadyBaselinedDiagnostics: originalBaselinedErrorsForFile, - }; - }); + } }; diff --git a/packages/pyright-internal/src/common/diagnostic.ts b/packages/pyright-internal/src/common/diagnostic.ts index bc0c3cf36..9cedc442b 100644 --- a/packages/pyright-internal/src/common/diagnostic.ts +++ b/packages/pyright-internal/src/common/diagnostic.ts @@ -122,6 +122,7 @@ export namespace DiagnosticRelatedInfo { // Represents a single error or warning. export class Diagnostic { + baselineStatus: 'not baselined' | 'baselined' | 'baselined with hint' = 'not baselined'; private _actions: DiagnosticAction[] | undefined; private _rule: string | undefined; private _relatedInfo: DiagnosticRelatedInfo[] = []; diff --git a/packages/pyright-internal/src/languageServerBase.ts b/packages/pyright-internal/src/languageServerBase.ts index 4be6bf472..00d42f359 100644 --- a/packages/pyright-internal/src/languageServerBase.ts +++ b/packages/pyright-internal/src/languageServerBase.ts @@ -134,12 +134,7 @@ import { InitStatus, WellKnownWorkspaceKinds, Workspace, WorkspaceFactory } from import { githubRepo } from './constants'; import { SemanticTokensProvider, SemanticTokensProviderLegend } from './languageService/semanticTokensProvider'; import { RenameUsageFinder } from './analyzer/renameUsageFinder'; -import { - diagnosticsToBaseline, - filterOutBaselinedDiagnostics, - getBaselinedErrors, - writeBaselineFile, -} from './baseline'; +import { getBaselinedErrorsForFile, writeDiagnosticsToBaselineFile } from './baseline'; export abstract class LanguageServerBase implements LanguageServerInterface, Disposable { // We support running only one "find all reference" at a time. @@ -1164,29 +1159,25 @@ export abstract class LanguageServerBase implements LanguageServerInterface, Dis const fileUri = Uri.file(params.textDocument.uri, this.serviceProvider); const workspace = await this.getWorkspaceForFile(fileUri); const rootUri = workspace.rootUri!; - const baselineFile = getBaselinedErrors(rootUri); - const fileKey = rootUri.getRelativePath(fileUri)!; + const previouslyBaselinedErrors = getBaselinedErrorsForFile(rootUri, fileUri); const diagnosticsForFile = this.documentsWithDiagnostics[params.textDocument.uri]; + let filesWithDiagnostics: FileDiagnostics[]; if (diagnosticsForFile) { - const filteredDiagnostics = filterOutBaselinedDiagnostics(rootUri, [diagnosticsForFile])[0]; + filesWithDiagnostics = [diagnosticsForFile]; if ( - // no baseline file exists - !filteredDiagnostics.alreadyBaselinedDiagnostics || + // no baseline file exists or no baselined errors exist for this file + !previouslyBaselinedErrors.length || // there are new diagnostics that haven't been baselined, so we don't want to write them // because the user will have to either fix the diagnostics or explicitly write them to the // baseline themselves - diagnosticsForFile.diagnostics.length > filteredDiagnostics.alreadyBaselinedDiagnostics.length + diagnosticsForFile.diagnostics.length > previouslyBaselinedErrors.length ) { return; } - } - if (diagnosticsForFile) { - //cringe - baselineFile.files[fileKey] = diagnosticsToBaseline(rootUri, [diagnosticsForFile]).files[fileKey]; } else { - baselineFile.files[fileKey] = []; + filesWithDiagnostics = [{ diagnostics: [], fileUri, version: undefined }]; } - writeBaselineFile(rootUri, baselineFile); + writeDiagnosticsToBaselineFile(rootUri, filesWithDiagnostics, true); }; protected async onExecuteCommand( @@ -1259,12 +1250,10 @@ export abstract class LanguageServerBase implements LanguageServerInterface, Dis fs: FileSystem, fileDiagnostics: FileDiagnostics ): Promise { - const workspace = await this.getWorkspaceForFile(fileDiagnostics.fileUri); - const filteredDiagnostics = filterOutBaselinedDiagnostics(workspace.rootUri!, [fileDiagnostics])[0]; return { - uri: convertUriToLspUriString(fs, filteredDiagnostics.fileUri), - version: filteredDiagnostics.version, - diagnostics: this._convertDiagnostics(fs, filteredDiagnostics.diagnostics), + uri: convertUriToLspUriString(fs, fileDiagnostics.fileUri), + version: fileDiagnostics.version, + diagnostics: this._convertDiagnostics(fs, fileDiagnostics.diagnostics), }; } @@ -1439,60 +1428,67 @@ export abstract class LanguageServerBase implements LanguageServerInterface, Dis private _convertDiagnostics(fs: FileSystem, diags: AnalyzerDiagnostic[]): Diagnostic[] { const convertedDiags: Diagnostic[] = []; - - diags.forEach((diag) => { - const severity = convertCategoryToSeverity(diag.category); - const rule = diag.getRule(); - const code = this.getDiagCode(diag, rule); - const vsDiag = Diagnostic.create(diag.range, diag.message, severity, code, this.serverOptions.productName); - - if ( - diag.category === DiagnosticCategory.UnusedCode || - diag.category === DiagnosticCategory.UnreachableCode - ) { - vsDiag.tags = [DiagnosticTag.Unnecessary]; - vsDiag.severity = DiagnosticSeverity.Hint; - - // If the client doesn't support "unnecessary" tags, don't report unused code. - if (!this.client.supportsUnnecessaryDiagnosticTag) { + diags + .filter((diag) => diag.baselineStatus !== 'baselined') + .forEach((diag) => { + const severity = convertCategoryToSeverity(diag.category); + const rule = diag.getRule(); + const code = this.getDiagCode(diag, rule); + const vsDiag = Diagnostic.create( + diag.range, + diag.message, + severity, + code, + this.serverOptions.productName + ); + + if ( + diag.category === DiagnosticCategory.UnusedCode || + diag.category === DiagnosticCategory.UnreachableCode + ) { + vsDiag.tags = [DiagnosticTag.Unnecessary]; + vsDiag.severity = DiagnosticSeverity.Hint; + + // If the client doesn't support "unnecessary" tags, don't report unused code. + if (!this.client.supportsUnnecessaryDiagnosticTag) { + return; + } + } else if (diag.category === DiagnosticCategory.Deprecated) { + vsDiag.tags = [DiagnosticTag.Deprecated]; + vsDiag.severity = DiagnosticSeverity.Hint; + + // If the client doesn't support "deprecated" tags, don't report. + if (!this.client.supportsDeprecatedDiagnosticTag) { + return; + } + } else if (diag.category === DiagnosticCategory.TaskItem) { + // TaskItem is not supported. return; } - } else if (diag.category === DiagnosticCategory.Deprecated) { - vsDiag.tags = [DiagnosticTag.Deprecated]; - vsDiag.severity = DiagnosticSeverity.Hint; - // If the client doesn't support "deprecated" tags, don't report. - if (!this.client.supportsDeprecatedDiagnosticTag) { - return; + if (rule) { + const ruleDocUrl = this.getDocumentationUrlForDiagnostic(diag); + if (ruleDocUrl) { + vsDiag.codeDescription = { + href: ruleDocUrl, + }; + } } - } else if (diag.category === DiagnosticCategory.TaskItem) { - // TaskItem is not supported. - return; - } - if (rule) { - const ruleDocUrl = this.getDocumentationUrlForDiagnostic(diag); - if (ruleDocUrl) { - vsDiag.codeDescription = { - href: ruleDocUrl, - }; + const relatedInfo = diag.getRelatedInfo(); + if (relatedInfo.length > 0) { + vsDiag.relatedInformation = relatedInfo + .filter((info) => this.canNavigateToFile(info.uri, fs)) + .map((info) => + DiagnosticRelatedInformation.create( + Location.create(convertUriToLspUriString(fs, info.uri), info.range), + info.message + ) + ); } - } - const relatedInfo = diag.getRelatedInfo(); - if (relatedInfo.length > 0) { - vsDiag.relatedInformation = relatedInfo - .filter((info) => this.canNavigateToFile(info.uri, fs)) - .map((info) => - DiagnosticRelatedInformation.create( - Location.create(convertUriToLspUriString(fs, info.uri), info.range), - info.message - ) - ); - } - - convertedDiags.push(vsDiag); - }); + convertedDiags.push(vsDiag); + }); function convertCategoryToSeverity(category: DiagnosticCategory) { switch (category) { diff --git a/packages/pyright-internal/src/pyright.ts b/packages/pyright-internal/src/pyright.ts index 233ef3c6d..42b683130 100644 --- a/packages/pyright-internal/src/pyright.ts +++ b/packages/pyright-internal/src/pyright.ts @@ -48,7 +48,7 @@ import * as core from '@actions/core'; import * as command from '@actions/core/lib/command'; import { convertDiagnostics } from 'pyright-to-gitlab-ci/src/converter'; import path from 'path'; -import { baselineFilePath, filterOutBaselinedDiagnostics, writeDiagnosticsToBaselineFile } from './baseline'; +import { baselineFilePath, getBaselinedErrors, writeDiagnosticsToBaselineFile } from './baseline'; type SeverityLevel = 'error' | 'warning' | 'information'; @@ -479,23 +479,19 @@ async function runSingleThreaded( ? Uri.file(options.executionRoot ?? '', service.serviceProvider) : options.executionRoot; const allDiagnostics = results.diagnostics; - const filteredDiagnostics = filterOutBaselinedDiagnostics(rootDir, results.diagnostics); + const baselinedErrorCount = Object.values(getBaselinedErrors(rootDir).files).flatMap((file) => file).length; const newErrorCount = allDiagnostics .flatMap( (file) => - file.diagnostics.filter((diagnostic) => - [DiagnosticCategory.Error, DiagnosticCategory.Warning, DiagnosticCategory.Information].includes( - diagnostic.category - ) + file.diagnostics.filter( + (diagnostic) => + !isHintDiagnostic(diagnostic) || diagnostic.baselineStatus === 'baselined with hint' ).length ) .reduce((prev, next) => prev + next); - if (args.writebaseline || !newErrorCount) { + const diff = newErrorCount - baselinedErrorCount; + if (args.writebaseline || diff < 0) { writeDiagnosticsToBaselineFile(rootDir, allDiagnostics, false); - const previousErrorCount = filteredDiagnostics - .flatMap((file) => file.alreadyBaselinedDiagnostics?.length ?? 0) - .reduce((prev, next) => prev + next); - const diff = newErrorCount - previousErrorCount; let message = ''; if (diff === 0) { message += "error count didn't change"; @@ -511,34 +507,37 @@ async function runSingleThreaded( )} (${message})` ); } - results.diagnostics = [...filteredDiagnostics]; // TODO: is this needed? + const filteredDiagnostics = results.diagnostics.map((file) => ({ + ...file, + diagnostics: file.diagnostics.filter((diagnostic) => diagnostic.baselineStatus === 'not baselined'), + })); let errorCount = 0; if (!args.createstub && !args.verifytypes) { let report: DiagnosticResult; if (args.outputjson) { report = reportDiagnosticsAsJson( - results.diagnostics, + filteredDiagnostics, minSeverityLevel, results.filesInProgram, results.elapsedTime ); } else if (process.env['GITHUB_ACTIONS'] && !process.env['PYRIGHT_DISABLE_GITHUB_ACTIONS_OUTPUT']) { report = reportDiagnosticsAsGithubActionsCommands( - results.diagnostics, + filteredDiagnostics, minSeverityLevel, results.filesInProgram, results.elapsedTime ); } else { printVersion(output); - report = reportDiagnosticsAsText(results.diagnostics, minSeverityLevel); + report = reportDiagnosticsAsText(filteredDiagnostics, minSeverityLevel); } if (args.gitlabcodequality) { fs.writeFileSync( args.gitlabcodequality, JSON.stringify( createGitlabCodeQualityReport( - results.diagnostics, + filteredDiagnostics, minSeverityLevel, results.filesInProgram, results.elapsedTime @@ -1195,7 +1194,7 @@ function printVersion(console: ConsoleInterface) { } function reportDiagnosticsAsJsonWithoutLogging( - fileDiagnostics: FileDiagnostics[], + fileDiagnostics: readonly FileDiagnostics[], minSeverityLevel: SeverityLevel, filesInProgram: number, timeInSec: number @@ -1241,7 +1240,7 @@ const pyrightJsonResultsToDiagnosticResult = (report: PyrightJsonResults): Diagn }); const reportDiagnosticsAsJson = ( - fileDiagnostics: FileDiagnostics[], + fileDiagnostics: readonly FileDiagnostics[], minSeverityLevel: SeverityLevel, filesInProgram: number, timeInSec: number @@ -1311,8 +1310,13 @@ const printDiagnosticSummary = (result: DiagnosticResult) => { ); }; +const isHintDiagnostic = (diagnostic: Diagnostic) => + diagnostic.category === DiagnosticCategory.UnusedCode || + diagnostic.category === DiagnosticCategory.UnreachableCode || + diagnostic.category === DiagnosticCategory.Deprecated; + function reportDiagnosticsAsText( - fileDiagnostics: FileDiagnostics[], + fileDiagnostics: readonly FileDiagnostics[], minSeverityLevel: SeverityLevel ): DiagnosticResult { let errorCount = 0; @@ -1324,9 +1328,7 @@ function reportDiagnosticsAsText( const fileErrorsAndWarnings = fileDiagnostics.diagnostics .filter( (diag) => - diag.category !== DiagnosticCategory.UnusedCode && - diag.category !== DiagnosticCategory.UnreachableCode && - diag.category !== DiagnosticCategory.Deprecated && + !isHintDiagnostic(diag) && isDiagnosticIncluded(convertDiagnosticCategoryToSeverity(diag.category), minSeverityLevel) ) .sort(compareDiagnostics); @@ -1384,7 +1386,7 @@ const diagnosticToString = (diagnostic: PyrightJsonDiagnostic, forCommand: boole }; const reportDiagnosticsAsGithubActionsCommands = ( - fileDiagnostics: FileDiagnostics[], + fileDiagnostics: readonly FileDiagnostics[], minSeverityLevel: SeverityLevel, filesInProgram: number, timeInSec: number @@ -1427,7 +1429,7 @@ const reportDiagnosticsAsGithubActionsCommands = ( }; const createGitlabCodeQualityReport = ( - fileDiagnostics: FileDiagnostics[], + fileDiagnostics: readonly FileDiagnostics[], minSeverityLevel: SeverityLevel, filesInProgram: number, timeInSec: number diff --git a/packages/pyright-internal/src/tests/baseline.test.ts b/packages/pyright-internal/src/tests/baseline.test.ts new file mode 100644 index 000000000..f1635ca2f --- /dev/null +++ b/packages/pyright-internal/src/tests/baseline.test.ts @@ -0,0 +1,13 @@ +import { ConfigOptions } from '../common/configOptions'; +import { DiagnosticRule } from '../common/diagnosticRules'; +import { Uri } from '../common/uri/uri'; +import { typeAnalyzeSampleFiles, validateResultsButBased } from './testUtils'; + +test('baselined error not reported', () => { + const analysisResults = typeAnalyzeSampleFiles( + ['baseline/baselined_error_not_reported/foo.py'], + new ConfigOptions(Uri.file(process.cwd(), serviceProvider)) + ); + + validateResultsButBased(analysisResults, { errors: [{ line: 1, code: DiagnosticRule.reportUndefinedVariable }] }); +}); diff --git a/packages/pyright-internal/src/tests/samples/baseline/baselined_error_not_reported/.basedpyright/baseline.json b/packages/pyright-internal/src/tests/samples/baseline/baselined_error_not_reported/.basedpyright/baseline.json new file mode 100644 index 000000000..055be3343 --- /dev/null +++ b/packages/pyright-internal/src/tests/samples/baseline/baselined_error_not_reported/.basedpyright/baseline.json @@ -0,0 +1,19 @@ +{ + "files": { + "./foo.py": [ + { + "code": "reportAssignmentType", + "range": { + "start": { + "line": 0, + "character": 11 + }, + "end": { + "line": 0, + "character": 13 + } + } + } + ] + } +} \ No newline at end of file diff --git a/packages/pyright-internal/src/tests/samples/baseline/baselined_error_not_reported/foo.py b/packages/pyright-internal/src/tests/samples/baseline/baselined_error_not_reported/foo.py new file mode 100644 index 000000000..34f6960cc --- /dev/null +++ b/packages/pyright-internal/src/tests/samples/baseline/baselined_error_not_reported/foo.py @@ -0,0 +1,2 @@ +foo: int = "" +asdf \ No newline at end of file From fa8ad63924f5f671b06b1f43939887d4fd62a31c Mon Sep 17 00:00:00 2001 From: detachhead Date: Mon, 16 Sep 2024 23:06:40 +1000 Subject: [PATCH 15/28] fix baseline test --- .../pyright-internal/src/common/diagnostic.ts | 4 +++- packages/pyright-internal/src/pyright.ts | 2 +- .../src/tests/baseline.test.ts | 24 ++++++++++++++----- .../pyright-internal/src/tests/testUtils.ts | 22 ++++++++++++----- 4 files changed, 38 insertions(+), 14 deletions(-) diff --git a/packages/pyright-internal/src/common/diagnostic.ts b/packages/pyright-internal/src/common/diagnostic.ts index 9cedc442b..48d0e401c 100644 --- a/packages/pyright-internal/src/common/diagnostic.ts +++ b/packages/pyright-internal/src/common/diagnostic.ts @@ -120,9 +120,11 @@ export namespace DiagnosticRelatedInfo { } } +export type BaselineStatus = 'baselined' | 'baselined with hint'; + // Represents a single error or warning. export class Diagnostic { - baselineStatus: 'not baselined' | 'baselined' | 'baselined with hint' = 'not baselined'; + baselineStatus?: BaselineStatus; private _actions: DiagnosticAction[] | undefined; private _rule: string | undefined; private _relatedInfo: DiagnosticRelatedInfo[] = []; diff --git a/packages/pyright-internal/src/pyright.ts b/packages/pyright-internal/src/pyright.ts index 42b683130..d4233247d 100644 --- a/packages/pyright-internal/src/pyright.ts +++ b/packages/pyright-internal/src/pyright.ts @@ -509,7 +509,7 @@ async function runSingleThreaded( } const filteredDiagnostics = results.diagnostics.map((file) => ({ ...file, - diagnostics: file.diagnostics.filter((diagnostic) => diagnostic.baselineStatus === 'not baselined'), + diagnostics: file.diagnostics.filter((diagnostic) => !diagnostic.baselineStatus), })); let errorCount = 0; if (!args.createstub && !args.verifytypes) { diff --git a/packages/pyright-internal/src/tests/baseline.test.ts b/packages/pyright-internal/src/tests/baseline.test.ts index f1635ca2f..85b8272c2 100644 --- a/packages/pyright-internal/src/tests/baseline.test.ts +++ b/packages/pyright-internal/src/tests/baseline.test.ts @@ -1,13 +1,25 @@ +import path from 'path'; import { ConfigOptions } from '../common/configOptions'; import { DiagnosticRule } from '../common/diagnosticRules'; import { Uri } from '../common/uri/uri'; -import { typeAnalyzeSampleFiles, validateResultsButBased } from './testUtils'; +import { resolveSampleFilePath, typeAnalyzeSampleFiles, validateResultsButBased } from './testUtils'; -test('baselined error not reported', () => { - const analysisResults = typeAnalyzeSampleFiles( - ['baseline/baselined_error_not_reported/foo.py'], - new ConfigOptions(Uri.file(process.cwd(), serviceProvider)) +const typeAnalyzeFilesWithBaseline = (sampleFolderName: string, files: string[]) => { + const sampleFolder = path.join('baseline', sampleFolderName); + return typeAnalyzeSampleFiles( + files.map((file) => path.join(sampleFolder, file)), + (serviceProvider) => new ConfigOptions(Uri.file(resolveSampleFilePath(sampleFolder), serviceProvider)) ); +}; + +test('baselined error not reported', () => { + const analysisResults = typeAnalyzeFilesWithBaseline('baselined_error_not_reported', ['foo.py']); - validateResultsButBased(analysisResults, { errors: [{ line: 1, code: DiagnosticRule.reportUndefinedVariable }] }); + validateResultsButBased(analysisResults, { + errors: [ + { line: 0, code: DiagnosticRule.reportAssignmentType, baselineStatus: 'baselined' }, + { line: 1, code: DiagnosticRule.reportUndefinedVariable }, + ], + warnings: [{ line: 1, code: DiagnosticRule.reportUnusedExpression }], + }); }); diff --git a/packages/pyright-internal/src/tests/testUtils.ts b/packages/pyright-internal/src/tests/testUtils.ts index 7172bd987..56034c2bd 100644 --- a/packages/pyright-internal/src/tests/testUtils.ts +++ b/packages/pyright-internal/src/tests/testUtils.ts @@ -18,7 +18,7 @@ import { TypeEvaluator } from '../analyzer/typeEvaluatorTypes'; import { ConfigOptions, ExecutionEnvironment, getStandardDiagnosticRuleSet } from '../common/configOptions'; import { ConsoleWithLogLevel, NullConsole } from '../common/console'; import { fail } from '../common/debug'; -import { Diagnostic, DiagnosticCategory } from '../common/diagnostic'; +import { BaselineStatus, Diagnostic, DiagnosticCategory } from '../common/diagnostic'; import { DiagnosticSink } from '../common/diagnosticSink'; import { FullAccessHost } from '../common/fullAccessHost'; import { RealTempFile, createFromRealFileSystem } from '../common/realFileSystem'; @@ -31,6 +31,7 @@ import { DiagnosticRule } from '../common/diagnosticRules'; import { SemanticTokenItem, SemanticTokensWalker } from '../analyzer/semanticTokensWalker'; import { TypeInlayHintsItemType, TypeInlayHintsWalker } from '../analyzer/typeInlayHintsWalker'; import { Range } from 'vscode-languageserver-types'; +import { ServiceProvider } from '../common/serviceProvider'; // This is a bit gross, but it's necessary to allow the fallback typeshed // directory to be located when running within the jest environment. This @@ -94,13 +95,20 @@ export function parseSampleFile( return parseText(text, diagSink); } -const createProgram = (configOptions = new ConfigOptions(Uri.empty()), console?: ConsoleWithLogLevel) => { - // Always enable "test mode". - configOptions.internalTestMode = true; +type ConfigOptionsArg = ConfigOptions | ((serviceProvider: ServiceProvider) => ConfigOptions); +const createProgram = ( + configOptions: ConfigOptionsArg = new ConfigOptions(Uri.empty()), + console?: ConsoleWithLogLevel +) => { const tempFile = new RealTempFile(); const fs = createFromRealFileSystem(tempFile); const serviceProvider = createServiceProvider(fs, console || new NullConsole(), tempFile); + if (typeof configOptions === 'function') { + configOptions = configOptions(serviceProvider); + } + // Always enable "test mode". + configOptions.internalTestMode = true; const importResolver = new ImportResolver(serviceProvider, configOptions, new FullAccessHost(serviceProvider)); return new Program(importResolver, configOptions, serviceProvider); @@ -108,7 +116,7 @@ const createProgram = (configOptions = new ConfigOptions(Uri.empty()), console?: export function typeAnalyzeSampleFiles( fileNames: string[], - configOptions = new ConfigOptions(Uri.empty()), + configOptions: ConfigOptionsArg = new ConfigOptions(Uri.empty()), console?: ConsoleWithLogLevel ): FileAnalysisResult[] { const program = createProgram(configOptions, console); @@ -123,7 +131,7 @@ export function typeAnalyzeSampleFiles( nameTypeWalker.walk(parserOutput.parseTree); }); - const results = getAnalysisResults(program, fileUris, configOptions); + const results = getAnalysisResults(program, fileUris, program.configOptions); program.dispose(); return results; @@ -246,6 +254,7 @@ interface ExpectedResult { message?: string; line: number; code?: DiagnosticRule; + baselineStatus?: BaselineStatus; } export type ExpectedResults = { @@ -264,6 +273,7 @@ export const validateResultsButBased = (allResults: FileAnalysisResult[], expect message: result.message, line: result.range.start.line, code: result.getRule() as DiagnosticRule | undefined, + baselineStatus: result.baselineStatus, }) ); const expectedResult = expectedResults[diagnosticType] ?? []; From 7a2da0a827cc6e08a974b7541fb943f57abbcdce Mon Sep 17 00:00:00 2001 From: detachhead Date: Tue, 17 Sep 2024 23:00:07 +1000 Subject: [PATCH 16/28] fix baseline being automatically rewritten when error count goes down but there are different errors --- .../src/languageServerBase.ts | 4 ++-- packages/pyright-internal/src/pyright.ts | 23 ++++++++++++------- 2 files changed, 17 insertions(+), 10 deletions(-) diff --git a/packages/pyright-internal/src/languageServerBase.ts b/packages/pyright-internal/src/languageServerBase.ts index 00d42f359..15988259c 100644 --- a/packages/pyright-internal/src/languageServerBase.ts +++ b/packages/pyright-internal/src/languageServerBase.ts @@ -1167,10 +1167,10 @@ export abstract class LanguageServerBase implements LanguageServerInterface, Dis if ( // no baseline file exists or no baselined errors exist for this file !previouslyBaselinedErrors.length || - // there are new diagnostics that haven't been baselined, so we don't want to write them + // there are diagnostics that haven't been baselined, so we don't want to write them // because the user will have to either fix the diagnostics or explicitly write them to the // baseline themselves - diagnosticsForFile.diagnostics.length > previouslyBaselinedErrors.length + diagnosticsForFile.diagnostics.some((diagnostic) => diagnostic.baselineStatus !== 'baselined') ) { return; } diff --git a/packages/pyright-internal/src/pyright.ts b/packages/pyright-internal/src/pyright.ts index d4233247d..4c7bbcb35 100644 --- a/packages/pyright-internal/src/pyright.ts +++ b/packages/pyright-internal/src/pyright.ts @@ -49,6 +49,7 @@ import * as command from '@actions/core/lib/command'; import { convertDiagnostics } from 'pyright-to-gitlab-ci/src/converter'; import path from 'path'; import { baselineFilePath, getBaselinedErrors, writeDiagnosticsToBaselineFile } from './baseline'; +import { add } from 'lodash'; type SeverityLevel = 'error' | 'warning' | 'information'; @@ -481,16 +482,26 @@ async function runSingleThreaded( const allDiagnostics = results.diagnostics; const baselinedErrorCount = Object.values(getBaselinedErrors(rootDir).files).flatMap((file) => file).length; const newErrorCount = allDiagnostics - .flatMap( + .map( (file) => file.diagnostics.filter( (diagnostic) => !isHintDiagnostic(diagnostic) || diagnostic.baselineStatus === 'baselined with hint' ).length ) - .reduce((prev, next) => prev + next); - const diff = newErrorCount - baselinedErrorCount; - if (args.writebaseline || diff < 0) { + .reduce(add); + const filteredDiagnostics = results.diagnostics.map((file) => ({ + ...file, + diagnostics: file.diagnostics.filter((diagnostic) => !diagnostic.baselineStatus), + })); + + // if there are any unbaselined errors, don't write to the baseline unless the user explicitly passed + // --writebaseline + if ( + args.writebaseline || + !filteredDiagnostics.map((fileWithDiagnostics) => fileWithDiagnostics.diagnostics.length).reduce(add) + ) { + const diff = newErrorCount - baselinedErrorCount; writeDiagnosticsToBaselineFile(rootDir, allDiagnostics, false); let message = ''; if (diff === 0) { @@ -507,10 +518,6 @@ async function runSingleThreaded( )} (${message})` ); } - const filteredDiagnostics = results.diagnostics.map((file) => ({ - ...file, - diagnostics: file.diagnostics.filter((diagnostic) => !diagnostic.baselineStatus), - })); let errorCount = 0; if (!args.createstub && !args.verifytypes) { let report: DiagnosticResult; From d90adb98f42886998d15ae8d62a45ac9df76bba3 Mon Sep 17 00:00:00 2001 From: detachhead Date: Fri, 20 Sep 2024 00:24:50 +1000 Subject: [PATCH 17/28] don't store columns in baseline because they aren't used for matching --- packages/pyright-internal/src/baseline.ts | 9 ++++----- .../.basedpyright/baseline.json | 10 ++-------- 2 files changed, 6 insertions(+), 13 deletions(-) diff --git a/packages/pyright-internal/src/baseline.ts b/packages/pyright-internal/src/baseline.ts index 7fb31de41..06e873371 100644 --- a/packages/pyright-internal/src/baseline.ts +++ b/packages/pyright-internal/src/baseline.ts @@ -1,6 +1,5 @@ import { DiagnosticRule } from './common/diagnosticRules'; import { FileDiagnostics } from './common/diagnosticSink'; -import { Range } from './common/textRange'; import { mkdirSync, readFileSync, writeFileSync } from 'fs'; import { Uri } from './common/uri/uri'; import { convertLevelToCategory, Diagnostic, DiagnosticCategory } from './common/diagnostic'; @@ -8,7 +7,7 @@ import { extraOptionDiagnosticRules } from './common/configOptions'; interface BaselinedDiagnostic { code: DiagnosticRule | undefined; - range: Range; + range: { startColumn: number; endColumn: number }; } interface BaselineFile { @@ -42,7 +41,7 @@ const diagnosticsToBaseline = (rootDir: Uri, filesWithDiagnostics: readonly File baselineData.files[filePath].push( ...errorDiagnostics.map((diagnostic) => ({ code: diagnostic.getRule() as DiagnosticRule | undefined, - range: diagnostic.range, + range: { startColumn: diagnostic.range.start.character, endColumn: diagnostic.range.end.character }, })) ); } @@ -106,8 +105,8 @@ export const filterOutBaselinedDiagnostics = (rootDir: Uri, file: Uri, diagnosti const matchedIndex = baselinedErrorsForFile.findIndex( (baselinedError) => baselinedError.code === diagnosticRule && - baselinedError.range.start.character === diagnostic.range.start.character && - baselinedError.range.end.character === diagnostic.range.end.character + baselinedError.range.startColumn === diagnostic.range.start.character && + baselinedError.range.endColumn === diagnostic.range.end.character ); if (matchedIndex >= 0) { baselinedErrorsForFile.splice(matchedIndex, 1); diff --git a/packages/pyright-internal/src/tests/samples/baseline/baselined_error_not_reported/.basedpyright/baseline.json b/packages/pyright-internal/src/tests/samples/baseline/baselined_error_not_reported/.basedpyright/baseline.json index 055be3343..c736a6168 100644 --- a/packages/pyright-internal/src/tests/samples/baseline/baselined_error_not_reported/.basedpyright/baseline.json +++ b/packages/pyright-internal/src/tests/samples/baseline/baselined_error_not_reported/.basedpyright/baseline.json @@ -4,14 +4,8 @@ { "code": "reportAssignmentType", "range": { - "start": { - "line": 0, - "character": 11 - }, - "end": { - "line": 0, - "character": 13 - } + "startColumn": 11, + "endColumn": 13 } } ] From 53ca1e7a917230d2156313445decc13ee4d02cb6 Mon Sep 17 00:00:00 2001 From: detachhead Date: Fri, 20 Sep 2024 22:03:06 +1000 Subject: [PATCH 18/28] rename vscode command category from pyright to basedpyright --- packages/vscode-pyright/package.json | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/packages/vscode-pyright/package.json b/packages/vscode-pyright/package.json index 1071f8a5b..0b20990cc 100644 --- a/packages/vscode-pyright/package.json +++ b/packages/vscode-pyright/package.json @@ -46,47 +46,47 @@ { "command": "basedpyright.organizeimports", "title": "Organize Imports", - "category": "Pyright" + "category": "basedpyright" }, { "command": "basedpyright.restartserver", "title": "Restart Server", - "category": "Pyright" + "category": "basedpyright" }, { "command": "basedpyright.dumpTokens", "title": "Dump token streams ...", - "category": "Pyright", + "category": "basedpyright", "enablement": "editorLangId == python && pyright.development" }, { "command": "basedpyright.dumpNodes", "title": "Dump parse tree ...", - "category": "Pyright", + "category": "basedpyright", "enablement": "editorLangId == python && pyright.development" }, { "command": "basedpyright.dumpTypes", "title": "Dump type info ...", - "category": "Pyright", + "category": "basedpyright", "enablement": "editorLangId == python && pyright.development" }, { "command": "basedpyright.dumpCachedTypes", "title": "Dump cached type info ...", - "category": "Pyright", + "category": "basedpyright", "enablement": "editorLangId == python && pyright.development" }, { "command": "basedpyright.dumpCodeFlowGraph", "title": "Dump code flow graph for node ...", - "category": "Pyright", + "category": "basedpyright", "enablement": "editorLangId == python && pyright.development" }, { "command": "basedpyright.writeBaseline", "title": "Write new errors to baseline", - "category": "Pyright" + "category": "basedpyright" } ], "menus": { From e292113b51bc093db67c7c8a10a9793241bc9be6 Mon Sep 17 00:00:00 2001 From: detachhead Date: Sat, 21 Sep 2024 16:29:09 +1000 Subject: [PATCH 19/28] remove duplicated code from `runSingleThreaded` and `runMultiThreaded` so now `--writebaseline` and `--gitlabcodequality` work with `--threads` --- packages/pyright-internal/src/pyright.ts | 209 +++++++++++------------ 1 file changed, 104 insertions(+), 105 deletions(-) diff --git a/packages/pyright-internal/src/pyright.ts b/packages/pyright-internal/src/pyright.ts index 4c7bbcb35..e86d58a39 100644 --- a/packages/pyright-internal/src/pyright.ts +++ b/packages/pyright-internal/src/pyright.ts @@ -449,6 +449,97 @@ async function processArgs(): Promise { return runSingleThreaded(args, options, service, minSeverityLevel, output); } +const outputResults = ( + args: CommandLineOptions, + options: PyrightCommandLineOptions, + results: Pick, + service: AnalyzerService, + minSeverityLevel: SeverityLevel, + output: ConsoleInterface +) => { + const rootDir = + typeof options.executionRoot === 'string' || options.executionRoot === undefined + ? Uri.file(options.executionRoot ?? '', service.serviceProvider) + : options.executionRoot; + const baselinedErrorCount = Object.values(getBaselinedErrors(rootDir).files).flatMap((file) => file).length; + const newErrorCount = results.diagnostics + .map( + (file) => + file.diagnostics.filter( + (diagnostic) => !isHintDiagnostic(diagnostic) || diagnostic.baselineStatus === 'baselined with hint' + ).length + ) + .reduce(add); + const filteredDiagnostics = results.diagnostics.map((file) => ({ + ...file, + diagnostics: file.diagnostics.filter((diagnostic) => !diagnostic.baselineStatus), + })); + + // if there are any unbaselined errors, don't write to the baseline unless the user explicitly passed + // --writebaseline + if ( + args.writebaseline || + !filteredDiagnostics.map((fileWithDiagnostics) => fileWithDiagnostics.diagnostics.length).reduce(add) + ) { + const diff = newErrorCount - baselinedErrorCount; + writeDiagnosticsToBaselineFile(rootDir, results.diagnostics, false); + let message = ''; + if (diff === 0) { + message += "error count didn't change"; + } else if (diff > 0) { + message += `went up by ${diff}`; + } else { + message += `went down by ${diff * -1}`; + } + console.info( + `updated ${rootDir.getRelativePath(baselineFilePath(rootDir))} with ${pluralize( + newErrorCount, + 'error' + )} (${message})` + ); + } + + const treatWarningsAsErrors = !!args.warnings; + let errorCount = 0; + let report: DiagnosticResult; + if (args.outputjson) { + report = reportDiagnosticsAsJson( + filteredDiagnostics, + minSeverityLevel, + results.filesInProgram, + results.elapsedTime + ); + } else if (process.env['GITHUB_ACTIONS'] && !process.env['PYRIGHT_DISABLE_GITHUB_ACTIONS_OUTPUT']) { + report = reportDiagnosticsAsGithubActionsCommands( + filteredDiagnostics, + minSeverityLevel, + results.filesInProgram, + results.elapsedTime + ); + } else { + printVersion(output); + report = reportDiagnosticsAsText(filteredDiagnostics, minSeverityLevel); + } + if (args.gitlabcodequality) { + fs.writeFileSync( + args.gitlabcodequality, + JSON.stringify( + createGitlabCodeQualityReport( + filteredDiagnostics, + minSeverityLevel, + results.filesInProgram, + results.elapsedTime + ) + ) + ); + } + errorCount += report.errorCount; + if (treatWarningsAsErrors) { + errorCount += report.warningCount; + } + return errorCount; +}; + async function runSingleThreaded( args: CommandLineOptions, options: PyrightCommandLineOptions, @@ -457,7 +548,6 @@ async function runSingleThreaded( output: ConsoleInterface ) { const watch = args.watch !== undefined; - const treatWarningsAsErrors = !!args.warnings; const exitStatus = createDeferred(); @@ -475,88 +565,10 @@ async function runSingleThreaded( return; } - const rootDir = - typeof options.executionRoot === 'string' || options.executionRoot === undefined - ? Uri.file(options.executionRoot ?? '', service.serviceProvider) - : options.executionRoot; - const allDiagnostics = results.diagnostics; - const baselinedErrorCount = Object.values(getBaselinedErrors(rootDir).files).flatMap((file) => file).length; - const newErrorCount = allDiagnostics - .map( - (file) => - file.diagnostics.filter( - (diagnostic) => - !isHintDiagnostic(diagnostic) || diagnostic.baselineStatus === 'baselined with hint' - ).length - ) - .reduce(add); - const filteredDiagnostics = results.diagnostics.map((file) => ({ - ...file, - diagnostics: file.diagnostics.filter((diagnostic) => !diagnostic.baselineStatus), - })); - - // if there are any unbaselined errors, don't write to the baseline unless the user explicitly passed - // --writebaseline - if ( - args.writebaseline || - !filteredDiagnostics.map((fileWithDiagnostics) => fileWithDiagnostics.diagnostics.length).reduce(add) - ) { - const diff = newErrorCount - baselinedErrorCount; - writeDiagnosticsToBaselineFile(rootDir, allDiagnostics, false); - let message = ''; - if (diff === 0) { - message += "error count didn't change"; - } else if (diff > 0) { - message += `went up by ${diff}`; - } else { - message += `went down by ${diff * -1}`; - } - console.info( - `updated ${rootDir.getRelativePath(baselineFilePath(rootDir))} with ${pluralize( - newErrorCount, - 'error' - )} (${message})` - ); - } - let errorCount = 0; - if (!args.createstub && !args.verifytypes) { - let report: DiagnosticResult; - if (args.outputjson) { - report = reportDiagnosticsAsJson( - filteredDiagnostics, - minSeverityLevel, - results.filesInProgram, - results.elapsedTime - ); - } else if (process.env['GITHUB_ACTIONS'] && !process.env['PYRIGHT_DISABLE_GITHUB_ACTIONS_OUTPUT']) { - report = reportDiagnosticsAsGithubActionsCommands( - filteredDiagnostics, - minSeverityLevel, - results.filesInProgram, - results.elapsedTime - ); - } else { - printVersion(output); - report = reportDiagnosticsAsText(filteredDiagnostics, minSeverityLevel); - } - if (args.gitlabcodequality) { - fs.writeFileSync( - args.gitlabcodequality, - JSON.stringify( - createGitlabCodeQualityReport( - filteredDiagnostics, - minSeverityLevel, - results.filesInProgram, - results.elapsedTime - ) - ) - ); - } - errorCount += report.errorCount; - if (treatWarningsAsErrors) { - errorCount += report.warningCount; - } - } + const errorCount = + args.createstub || args.verifytypes + ? 0 + : outputResults(args, options, results, service, minSeverityLevel, output); if (args.createstub && results.requiringAnalysisCount.files === 0) { try { @@ -622,7 +634,6 @@ async function runMultiThreaded( ) { const workers: ChildProcess[] = []; const startTime = Date.now(); - const treatWarningsAsErrors = !!args.warnings; const exitStatus = createDeferred(); // Specify that only open files should be checked. This will allow us @@ -694,27 +705,15 @@ async function runMultiThreaded( // is complete, report the results and exit. if (!exitStatus.resolved) { const elapsedTime = (Date.now() - startTime) / 1000; - let errorCount = 0; - - if (args.outputjson) { - const report = reportDiagnosticsAsJson( - fileDiagnostics, - minSeverityLevel, - sourceFilesToAnalyze.length, - elapsedTime - ); - errorCount += report.errorCount; - if (treatWarningsAsErrors) { - errorCount += report.warningCount; - } - } else { - printVersion(output); - const report = reportDiagnosticsAsText(fileDiagnostics, minSeverityLevel); - errorCount += report.errorCount; - if (treatWarningsAsErrors) { - errorCount += report.warningCount; - } - + const errorCount = outputResults( + args, + options, + { diagnostics: fileDiagnostics, filesInProgram: sourceFilesToAnalyze.length, elapsedTime }, + service, + minSeverityLevel, + output + ); + if (!args.outputjson) { // Print the total time. output.info(`Completed in ${elapsedTime}sec`); } From 3d968e224e8891ceb14e39ee312e41b134ef07ed Mon Sep 17 00:00:00 2001 From: detachhead Date: Sat, 21 Sep 2024 16:41:10 +1000 Subject: [PATCH 20/28] use exclude to prevent basedpyright self check from checking ignored vendored typeshed files --- pyproject.toml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index e0f22583d..4c15b879b 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -193,9 +193,11 @@ max-line-length = 200 [tool.basedpyright] pythonVersion = "3.8" include = ["basedpyright", "based_build", "pdm_build.py", "tests"] +# https://github.com/DetachHead/basedpyright/issues/31 ignore = ["pw", "basedpyright/dist", "packages", "build"] +exclude = ["pw", "basedpyright/dist", "packages", "build"] typeCheckingMode = "all" -reportImplicitStringConcatenation = false # conflicts with ruff formatter +reportImplicitStringConcatenation = false # conflicts with ruff formatter # currently commented out due to https://github.com/DetachHead/basedpyright/issues/570 # only seem to be needed when using pycharm, which i dont really care about anyway because pycharm sucks From fe62955f853de77cb47f1cc032f1cd6b424dddd4 Mon Sep 17 00:00:00 2001 From: detachhead Date: Sun, 22 Sep 2024 17:50:18 +1000 Subject: [PATCH 21/28] refactor baseline stuff to use pyright filesystem, move summary message code to its own function so the lsp command can use it too, and improve logic to find the workspace in the lsp command --- .../src/analyzer/sourceFile.ts | 2 +- packages/pyright-internal/src/baseline.ts | 84 ++++++++++++++----- .../src/commands/writeBaseline.ts | 40 +++++++-- .../src/common/stringUtils.ts | 3 + .../src/languageServerBase.ts | 4 +- packages/pyright-internal/src/pyright.ts | 35 ++------ 6 files changed, 108 insertions(+), 60 deletions(-) diff --git a/packages/pyright-internal/src/analyzer/sourceFile.ts b/packages/pyright-internal/src/analyzer/sourceFile.ts index 8c2dcb543..dc3542450 100644 --- a/packages/pyright-internal/src/analyzer/sourceFile.ts +++ b/packages/pyright-internal/src/analyzer/sourceFile.ts @@ -1247,7 +1247,7 @@ export class SourceFile { // Now add in the "unnecessary type ignore" diagnostics. diagList = diagList.concat(unnecessaryTypeIgnoreDiags); - filterOutBaselinedDiagnostics(configOptions.projectRoot, this._uri, diagList); + filterOutBaselinedDiagnostics(this.fileSystem, configOptions.projectRoot, this._uri, diagList); // If we're not returning any diagnostics, filter out all of // the errors and warnings, leaving only the unreachable code diff --git a/packages/pyright-internal/src/baseline.ts b/packages/pyright-internal/src/baseline.ts index 06e873371..b991c3d98 100644 --- a/packages/pyright-internal/src/baseline.ts +++ b/packages/pyright-internal/src/baseline.ts @@ -1,9 +1,11 @@ import { DiagnosticRule } from './common/diagnosticRules'; import { FileDiagnostics } from './common/diagnosticSink'; -import { mkdirSync, readFileSync, writeFileSync } from 'fs'; import { Uri } from './common/uri/uri'; import { convertLevelToCategory, Diagnostic, DiagnosticCategory } from './common/diagnostic'; import { extraOptionDiagnosticRules } from './common/configOptions'; +import { fileExists } from './common/uri/uriUtils'; +import { FileSystem, ReadOnlyFileSystem } from './common/fileSystem'; +import { pluralize } from './common/stringUtils'; interface BaselinedDiagnostic { code: DiagnosticRule | undefined; @@ -48,57 +50,95 @@ const diagnosticsToBaseline = (rootDir: Uri, filesWithDiagnostics: readonly File return baselineData; }; -const writeBaselineFile = (rootDir: Uri, baselineData: BaselineFile) => { +const writeBaselineFile = (fileSystem: FileSystem, rootDir: Uri, baselineData: BaselineFile) => { const baselineFile = baselineFilePath(rootDir); - mkdirSync(baselineFile.getDirectory().getPath(), { recursive: true }); - writeFileSync(baselineFile.getPath(), JSON.stringify(baselineData, undefined, 4)); + fileSystem.mkdirSync(baselineFile.getDirectory(), { recursive: true }); + fileSystem.writeFileSync(baselineFile, JSON.stringify(baselineData, undefined, 4), null); }; /** - * @param openFilesOnly whether or not the diagnostics were only reported on the open files. setting this to `true` prevents - * it from deleting baselined errors from files that weren't opened + * @param openFilesOnly whether or not we know that the diagnostics were only reported on the open files. setting this + * to `true` prevents it from checking whether or not previously baselined files still exist, which probably makes it + * faster + * @returns the new contents of the baseline file */ -export const writeDiagnosticsToBaselineFile = async ( +export const writeDiagnosticsToBaselineFile = ( + fs: FileSystem, rootDir: Uri, filesWithDiagnostics: readonly FileDiagnostics[], openFilesOnly: boolean -) => { - let baselineData = diagnosticsToBaseline(rootDir, filesWithDiagnostics); - if (openFilesOnly) { - baselineData = { files: { ...getBaselinedErrors(rootDir).files, ...baselineData.files } }; +): BaselineFile => { + const newBaseline = diagnosticsToBaseline(rootDir, filesWithDiagnostics).files; + const previousBaseline = getBaselinedErrors(fs, rootDir).files; + // we don't know for sure that basedpyright was run on every file that was included when the previous baseline was + // generated, so we check previously baselined files that aren't in the new baseline to see if they still exist. if + // not, we assume the file was renamed or deleted and therefore its baseline entry should be removed. when + // `openFilesOnly` is `true` we skip the file exists check to make the langusge server faster because it's very + // likely that lots of files are missing from the new baseline. + for (const filePath in previousBaseline) { + if (!newBaseline[filePath] && (openFilesOnly || fileExists(fs, rootDir.combinePaths(filePath)))) { + newBaseline[filePath] = previousBaseline[filePath]; + } } // remove files where there are no errors - for (const file in baselineData.files) { - if (!baselineData.files[file].length) { - delete baselineData.files[file]; + for (const file in newBaseline) { + if (!newBaseline[file].length) { + delete newBaseline[file]; } } - writeBaselineFile(rootDir, baselineData); + const result = { files: newBaseline }; + writeBaselineFile(fs, rootDir, result); + return result; }; -export const getBaselinedErrors = (rootDir: Uri): BaselineFile => { - const path = baselineFilePath(rootDir).getPath(); +export const getBaselineSummaryMessage = (rootDir: Uri, previousBaseline: BaselineFile, newBaseline: BaselineFile) => { + const baselinedErrorCount = Object.values(previousBaseline.files).flatMap((file) => file).length; + const newErrorCount = Object.values(newBaseline.files).flatMap((file) => file).length; + + const diff = newErrorCount - baselinedErrorCount; + let message = ''; + if (diff === 0) { + message += "error count didn't change"; + } else if (diff > 0) { + message += `went up by ${diff}`; + } else { + message += `went down by ${diff * -1}`; + } + + return `updated ${rootDir.getRelativePath(baselineFilePath(rootDir))} with ${pluralize( + newErrorCount, + 'error' + )} (${message})`; +}; + +export const getBaselinedErrors = (fs: ReadOnlyFileSystem, rootDir: Uri): BaselineFile => { + const path = baselineFilePath(rootDir); let baselineFileContents; try { - baselineFileContents = readFileSync(path, 'utf8'); + baselineFileContents = fs.readFileSync(path, 'utf8'); } catch (e) { return { files: {} }; } return JSON.parse(baselineFileContents); }; -export const getBaselinedErrorsForFile = (rootDir: Uri, file: Uri): BaselinedDiagnostic[] => { +export const getBaselinedErrorsForFile = (fs: ReadOnlyFileSystem, rootDir: Uri, file: Uri): BaselinedDiagnostic[] => { const relativePath = rootDir.getRelativePath(file); let result; // if this is undefined it means the file isn't in the workspace if (relativePath) { - result = getBaselinedErrors(rootDir).files[rootDir.getRelativePath(file)!.toString()]; + result = getBaselinedErrors(fs, rootDir).files[rootDir.getRelativePath(file)!.toString()]; } return result ?? []; }; -export const filterOutBaselinedDiagnostics = (rootDir: Uri, file: Uri, diagnostics: Diagnostic[]) => { - const baselinedErrorsForFile = getBaselinedErrorsForFile(rootDir, file); +export const filterOutBaselinedDiagnostics = ( + fs: ReadOnlyFileSystem, + rootDir: Uri, + file: Uri, + diagnostics: Diagnostic[] +) => { + const baselinedErrorsForFile = getBaselinedErrorsForFile(fs, rootDir, file); for (const index in diagnostics) { const diagnostic = diagnostics[index]; const diagnosticRule = diagnostic.getRule() as DiagnosticRule | undefined; diff --git a/packages/pyright-internal/src/commands/writeBaseline.ts b/packages/pyright-internal/src/commands/writeBaseline.ts index ffd5f2a9b..641eb7d22 100644 --- a/packages/pyright-internal/src/commands/writeBaseline.ts +++ b/packages/pyright-internal/src/commands/writeBaseline.ts @@ -1,5 +1,10 @@ import { ServerCommand } from './commandController'; -import { writeDiagnosticsToBaselineFile } from '../baseline'; +import { + baselineFilePath, + getBaselinedErrors, + getBaselineSummaryMessage, + writeDiagnosticsToBaselineFile, +} from '../baseline'; import { LanguageServerInterface } from '../common/languageServerInterface'; export class WriteBaselineCommand implements ServerCommand { @@ -9,20 +14,43 @@ export class WriteBaselineCommand implements ServerCommand { async execute(): Promise { // TODO: figure out a better way to get workspace root - const firstFile = this._ls.documentsWithDiagnostics[Object.keys(this._ls.documentsWithDiagnostics)[0]]?.fileUri; - if (firstFile) { - const workspace = await this._ls.getWorkspaceForFile(firstFile); + // first we try and find the first workspace that has a baseline file and assume that's the right one + const workspaces = await this._ls.getWorkspaces(); + if (!workspaces.length) { + this._ls.window.showErrorMessage('cannot write to the baseline file because no workspace is open'); + return; + } + let workspace = workspaces.find((workspace) => + workspace.rootUri ? workspace.service.fs.existsSync(baselineFilePath(workspace.rootUri)) : false + ); + if (!workspace) { + // if there's no baseline file yet, we do it in an even hackier way, by getting the workspace from + // any open file that has diagnostics in it. + const firstFile = Object.values(this._ls.documentsWithDiagnostics)[0]?.fileUri; + if (firstFile) { + workspace = await this._ls.getWorkspaceForFile(firstFile); + } + } + if (workspace) { const workspaceRoot = workspace.rootUri; if (workspaceRoot) { - await writeDiagnosticsToBaselineFile( + const previousBaseline = getBaselinedErrors(workspace.service.fs, workspaceRoot); + const newBaseline = writeDiagnosticsToBaselineFile( + workspace.service.fs, workspaceRoot, Object.values(this._ls.documentsWithDiagnostics), true ); workspace.service.baselineUpdated(); + this._ls.window.showInformationMessage( + getBaselineSummaryMessage(workspaceRoot, previousBaseline, newBaseline) + ); return; } } - this._ls.window.showErrorMessage('cannot write to the baseline file because no workspace is open'); + // the only time the rootUri would not be found if there was no baseline file and no files with any + // diagnostics in them. this is because of the hacky method we use above to get the workspace. + // but we disguise this as an information message because it means we don't need to write anything anyway + this._ls.window.showInformationMessage('no baseline file was found and there are no diagnostics to baseline'); } } diff --git a/packages/pyright-internal/src/common/stringUtils.ts b/packages/pyright-internal/src/common/stringUtils.ts index e421d8bf5..20b90d935 100644 --- a/packages/pyright-internal/src/common/stringUtils.ts +++ b/packages/pyright-internal/src/common/stringUtils.ts @@ -192,3 +192,6 @@ export function escapeRegExp(text: string) { */ export const userFacingOptionsList = (values: string[]) => values.map((mode, index) => (index < values.length - 1 ? `"${mode}", ` : `or "${mode}"`)).join(''); + +export const pluralize = (n: number, singular: string, plural: string = `${singular}s`) => + `${n} ${n === 1 ? singular : plural}`; diff --git a/packages/pyright-internal/src/languageServerBase.ts b/packages/pyright-internal/src/languageServerBase.ts index 15988259c..c6a130ae9 100644 --- a/packages/pyright-internal/src/languageServerBase.ts +++ b/packages/pyright-internal/src/languageServerBase.ts @@ -1159,7 +1159,7 @@ export abstract class LanguageServerBase implements LanguageServerInterface, Dis const fileUri = Uri.file(params.textDocument.uri, this.serviceProvider); const workspace = await this.getWorkspaceForFile(fileUri); const rootUri = workspace.rootUri!; - const previouslyBaselinedErrors = getBaselinedErrorsForFile(rootUri, fileUri); + const previouslyBaselinedErrors = getBaselinedErrorsForFile(this.fs, rootUri, fileUri); const diagnosticsForFile = this.documentsWithDiagnostics[params.textDocument.uri]; let filesWithDiagnostics: FileDiagnostics[]; if (diagnosticsForFile) { @@ -1177,7 +1177,7 @@ export abstract class LanguageServerBase implements LanguageServerInterface, Dis } else { filesWithDiagnostics = [{ diagnostics: [], fileUri, version: undefined }]; } - writeDiagnosticsToBaselineFile(rootUri, filesWithDiagnostics, true); + writeDiagnosticsToBaselineFile(this.fs, rootUri, filesWithDiagnostics, true); }; protected async onExecuteCommand( diff --git a/packages/pyright-internal/src/pyright.ts b/packages/pyright-internal/src/pyright.ts index e86d58a39..899999879 100644 --- a/packages/pyright-internal/src/pyright.ts +++ b/packages/pyright-internal/src/pyright.ts @@ -48,8 +48,9 @@ import * as core from '@actions/core'; import * as command from '@actions/core/lib/command'; import { convertDiagnostics } from 'pyright-to-gitlab-ci/src/converter'; import path from 'path'; -import { baselineFilePath, getBaselinedErrors, writeDiagnosticsToBaselineFile } from './baseline'; +import { getBaselinedErrors, getBaselineSummaryMessage, writeDiagnosticsToBaselineFile } from './baseline'; import { add } from 'lodash'; +import { pluralize } from './common/stringUtils'; type SeverityLevel = 'error' | 'warning' | 'information'; @@ -461,15 +462,7 @@ const outputResults = ( typeof options.executionRoot === 'string' || options.executionRoot === undefined ? Uri.file(options.executionRoot ?? '', service.serviceProvider) : options.executionRoot; - const baselinedErrorCount = Object.values(getBaselinedErrors(rootDir).files).flatMap((file) => file).length; - const newErrorCount = results.diagnostics - .map( - (file) => - file.diagnostics.filter( - (diagnostic) => !isHintDiagnostic(diagnostic) || diagnostic.baselineStatus === 'baselined with hint' - ).length - ) - .reduce(add); + const filteredDiagnostics = results.diagnostics.map((file) => ({ ...file, diagnostics: file.diagnostics.filter((diagnostic) => !diagnostic.baselineStatus), @@ -481,22 +474,9 @@ const outputResults = ( args.writebaseline || !filteredDiagnostics.map((fileWithDiagnostics) => fileWithDiagnostics.diagnostics.length).reduce(add) ) { - const diff = newErrorCount - baselinedErrorCount; - writeDiagnosticsToBaselineFile(rootDir, results.diagnostics, false); - let message = ''; - if (diff === 0) { - message += "error count didn't change"; - } else if (diff > 0) { - message += `went up by ${diff}`; - } else { - message += `went down by ${diff * -1}`; - } - console.info( - `updated ${rootDir.getRelativePath(baselineFilePath(rootDir))} with ${pluralize( - newErrorCount, - 'error' - )} (${message})` - ); + const previousBaseline = getBaselinedErrors(service.fs, rootDir); + const newBaseline = writeDiagnosticsToBaselineFile(service.fs, rootDir, results.diagnostics, false); + console.info(getBaselineSummaryMessage(rootDir, previousBaseline, newBaseline)); } const treatWarningsAsErrors = !!args.warnings; @@ -1302,9 +1282,6 @@ function convertDiagnosticToJson(filePath: string, diag: Diagnostic): PyrightJso }; } -const pluralize = (n: number, singular: string, plural: string = `${singular}s`) => - `${n} ${n === 1 ? singular : plural}`; - const printDiagnosticSummary = (result: DiagnosticResult) => { console.info( [ From 8bf5b7673f9c6bfcd92249debd39c5d487a7babc Mon Sep 17 00:00:00 2001 From: detachhead Date: Sun, 22 Sep 2024 17:52:58 +1000 Subject: [PATCH 22/28] address potential concerns that this change doesnt have enough test coverage --- packages/pyright-internal/src/tests/baseline.test.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/pyright-internal/src/tests/baseline.test.ts b/packages/pyright-internal/src/tests/baseline.test.ts index 85b8272c2..33a270400 100644 --- a/packages/pyright-internal/src/tests/baseline.test.ts +++ b/packages/pyright-internal/src/tests/baseline.test.ts @@ -23,3 +23,5 @@ test('baselined error not reported', () => { warnings: [{ line: 1, code: DiagnosticRule.reportUnusedExpression }], }); }); + +// TODO: more tests From 188981f0b4b93175bf6afe742571f214e3e14be2 Mon Sep 17 00:00:00 2001 From: detachhead Date: Sun, 22 Sep 2024 19:17:10 +1000 Subject: [PATCH 23/28] fix cli crash when checking baseline file if there are no files --- packages/pyright-internal/src/pyright.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/pyright-internal/src/pyright.ts b/packages/pyright-internal/src/pyright.ts index 899999879..fab946e36 100644 --- a/packages/pyright-internal/src/pyright.ts +++ b/packages/pyright-internal/src/pyright.ts @@ -472,7 +472,7 @@ const outputResults = ( // --writebaseline if ( args.writebaseline || - !filteredDiagnostics.map((fileWithDiagnostics) => fileWithDiagnostics.diagnostics.length).reduce(add) + !filteredDiagnostics.map((fileWithDiagnostics) => fileWithDiagnostics.diagnostics.length).reduce(add, 0) ) { const previousBaseline = getBaselinedErrors(service.fs, rootDir); const newBaseline = writeDiagnosticsToBaselineFile(service.fs, rootDir, results.diagnostics, false); From a14539d4608e62813fda3989fdbf386ec5306c92 Mon Sep 17 00:00:00 2001 From: detachhead Date: Sun, 22 Sep 2024 20:54:56 +1000 Subject: [PATCH 24/28] sort baselined files before writing them to the baseline file to prevent needless diffs when the baseline is updated by the language server --- packages/pyright-internal/src/baseline.ts | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/packages/pyright-internal/src/baseline.ts b/packages/pyright-internal/src/baseline.ts index b991c3d98..7f7b75e27 100644 --- a/packages/pyright-internal/src/baseline.ts +++ b/packages/pyright-internal/src/baseline.ts @@ -50,12 +50,6 @@ const diagnosticsToBaseline = (rootDir: Uri, filesWithDiagnostics: readonly File return baselineData; }; -const writeBaselineFile = (fileSystem: FileSystem, rootDir: Uri, baselineData: BaselineFile) => { - const baselineFile = baselineFilePath(rootDir); - fileSystem.mkdirSync(baselineFile.getDirectory(), { recursive: true }); - fileSystem.writeFileSync(baselineFile, JSON.stringify(baselineData, undefined, 4), null); -}; - /** * @param openFilesOnly whether or not we know that the diagnostics were only reported on the open files. setting this * to `true` prevents it from checking whether or not previously baselined files still exist, which probably makes it @@ -80,14 +74,18 @@ export const writeDiagnosticsToBaselineFile = ( newBaseline[filePath] = previousBaseline[filePath]; } } - // remove files where there are no errors - for (const file in newBaseline) { - if (!newBaseline[file].length) { - delete newBaseline[file]; + const result: BaselineFile = { files: {} }; + // sort the file names so they always show up in the same order + // to prevent needless diffs between baseline files generated by the language server and the cli + for (const file of Object.keys(newBaseline).sort()) { + // remove files where there are no errors + if (newBaseline[file].length) { + result.files[file] = newBaseline[file]; } } - const result = { files: newBaseline }; - writeBaselineFile(fs, rootDir, result); + const baselineFile = baselineFilePath(rootDir); + fs.mkdirSync(baselineFile.getDirectory(), { recursive: true }); + fs.writeFileSync(baselineFile, JSON.stringify(result, undefined, 4), null); return result; }; From 75028edc3b8c504d46cf833fdcb5b0f6f6b2cb8a Mon Sep 17 00:00:00 2001 From: detachhead Date: Sun, 22 Sep 2024 22:39:48 +1000 Subject: [PATCH 25/28] prevent excluded files from getting baselined when using the write baseline lsp command --- .../pyright-internal/src/commands/writeBaseline.ts | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/packages/pyright-internal/src/commands/writeBaseline.ts b/packages/pyright-internal/src/commands/writeBaseline.ts index 641eb7d22..7abee69d8 100644 --- a/packages/pyright-internal/src/commands/writeBaseline.ts +++ b/packages/pyright-internal/src/commands/writeBaseline.ts @@ -6,6 +6,8 @@ import { writeDiagnosticsToBaselineFile, } from '../baseline'; import { LanguageServerInterface } from '../common/languageServerInterface'; +import { matchFileSpecs } from '../common/configOptions'; +import { Uri } from '../common/uri/uri'; export class WriteBaselineCommand implements ServerCommand { constructor(private _ls: LanguageServerInterface) { @@ -35,10 +37,16 @@ export class WriteBaselineCommand implements ServerCommand { const workspaceRoot = workspace.rootUri; if (workspaceRoot) { const previousBaseline = getBaselinedErrors(workspace.service.fs, workspaceRoot); + const configOptions = workspace.service.getConfigOptions(); + // filter out excluded files. ideally they shouldn't be present at all. see + // https://github.com/DetachHead/basedpyright/issues/31 + const filteredFiles = Object.entries(this._ls.documentsWithDiagnostics) + .filter(([filePath]) => matchFileSpecs(configOptions, Uri.file(filePath, this._ls.serviceProvider))) + .map(([_, diagnostics]) => diagnostics); const newBaseline = writeDiagnosticsToBaselineFile( workspace.service.fs, workspaceRoot, - Object.values(this._ls.documentsWithDiagnostics), + filteredFiles, true ); workspace.service.baselineUpdated(); From 14d8bc03bfef86580c476ba1de68503ca7087e95 Mon Sep 17 00:00:00 2001 From: detachhead Date: Tue, 24 Sep 2024 19:56:58 +1000 Subject: [PATCH 26/28] fix diagnostics not being deleted when workspace removed i think --- packages/pyright-internal/src/languageServerBase.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/pyright-internal/src/languageServerBase.ts b/packages/pyright-internal/src/languageServerBase.ts index c6a130ae9..b4007f533 100644 --- a/packages/pyright-internal/src/languageServerBase.ts +++ b/packages/pyright-internal/src/languageServerBase.ts @@ -1330,7 +1330,7 @@ export abstract class LanguageServerBase implements LanguageServerInterface, Dis } this.sendDiagnostics(this.fs, { fileUri: fileWithDiagnostics.fileUri, - diagnostics: fileWithDiagnostics.diagnostics, + diagnostics: [], version: undefined, }); } From 58a9f558bfb04793bdc856a8b46e1046d76c58f0 Mon Sep 17 00:00:00 2001 From: detachhead Date: Tue, 24 Sep 2024 19:57:44 +1000 Subject: [PATCH 27/28] fix baselined errors being incorrectly deleted if a file is saved before it gets analyzed --- packages/pyright-internal/src/baseline.ts | 2 +- .../src/languageServerBase.ts | 97 +++++++++++++------ 2 files changed, 67 insertions(+), 32 deletions(-) diff --git a/packages/pyright-internal/src/baseline.ts b/packages/pyright-internal/src/baseline.ts index 7f7b75e27..37ec3870b 100644 --- a/packages/pyright-internal/src/baseline.ts +++ b/packages/pyright-internal/src/baseline.ts @@ -7,7 +7,7 @@ import { fileExists } from './common/uri/uriUtils'; import { FileSystem, ReadOnlyFileSystem } from './common/fileSystem'; import { pluralize } from './common/stringUtils'; -interface BaselinedDiagnostic { +export interface BaselinedDiagnostic { code: DiagnosticRule | undefined; range: { startColumn: number; endColumn: number }; } diff --git a/packages/pyright-internal/src/languageServerBase.ts b/packages/pyright-internal/src/languageServerBase.ts index b4007f533..2d84a6e28 100644 --- a/packages/pyright-internal/src/languageServerBase.ts +++ b/packages/pyright-internal/src/languageServerBase.ts @@ -67,14 +67,19 @@ import { TextDocumentEdit, TextDocumentPositionParams, TextDocumentSyncKind, - WillSaveTextDocumentParams, WorkDoneProgressReporter, WorkspaceEdit, WorkspaceFoldersChangeEvent, WorkspaceSymbol, WorkspaceSymbolParams, } from 'vscode-languageserver'; -import { InlayHint, InlayHintParams, SemanticTokens, SemanticTokensParams } from 'vscode-languageserver-protocol'; +import { + InlayHint, + InlayHintParams, + SemanticTokens, + SemanticTokensParams, + WillSaveTextDocumentParams, +} from 'vscode-languageserver-protocol'; import { ResultProgressReporter } from 'vscode-languageserver'; import { TextDocument } from 'vscode-languageserver-textdocument'; @@ -134,7 +139,7 @@ import { InitStatus, WellKnownWorkspaceKinds, Workspace, WorkspaceFactory } from import { githubRepo } from './constants'; import { SemanticTokensProvider, SemanticTokensProviderLegend } from './languageService/semanticTokensProvider'; import { RenameUsageFinder } from './analyzer/renameUsageFinder'; -import { getBaselinedErrorsForFile, writeDiagnosticsToBaselineFile } from './baseline'; +import { BaselinedDiagnostic, getBaselinedErrorsForFile, writeDiagnosticsToBaselineFile } from './baseline'; export abstract class LanguageServerBase implements LanguageServerInterface, Disposable { // We support running only one "find all reference" at a time. @@ -181,8 +186,13 @@ export abstract class LanguageServerBase implements LanguageServerInterface, Dis protected readonly fs: FileSystem; protected readonly caseSensitiveDetector: CaseSensitivityDetector; + protected readonly savedFilesForBaselineUpdate = new Map< + Uri, + { workspace: Workspace; baseline: readonly BaselinedDiagnostic[] } + >(); + // The URIs for which diagnostics are reported - readonly documentsWithDiagnostics: Record = {}; + readonly documentsWithDiagnostics: Record = {}; protected readonly dynamicFeatures = new DynamicFeatures(); @@ -1155,29 +1165,19 @@ export abstract class LanguageServerBase implements LanguageServerInterface, Dis return result; }; + /** + * get the baselined errors for the file we just saved and tell the analysis complete handler + * that it may need to update the baseline for it + */ protected onSaveTextDocument = async (params: WillSaveTextDocumentParams) => { const fileUri = Uri.file(params.textDocument.uri, this.serviceProvider); const workspace = await this.getWorkspaceForFile(fileUri); - const rootUri = workspace.rootUri!; - const previouslyBaselinedErrors = getBaselinedErrorsForFile(this.fs, rootUri, fileUri); - const diagnosticsForFile = this.documentsWithDiagnostics[params.textDocument.uri]; - let filesWithDiagnostics: FileDiagnostics[]; - if (diagnosticsForFile) { - filesWithDiagnostics = [diagnosticsForFile]; - if ( - // no baseline file exists or no baselined errors exist for this file - !previouslyBaselinedErrors.length || - // there are diagnostics that haven't been baselined, so we don't want to write them - // because the user will have to either fix the diagnostics or explicitly write them to the - // baseline themselves - diagnosticsForFile.diagnostics.some((diagnostic) => diagnostic.baselineStatus !== 'baselined') - ) { - return; - } - } else { - filesWithDiagnostics = [{ diagnostics: [], fileUri, version: undefined }]; + if (workspace.rootUri) { + this.savedFilesForBaselineUpdate.set(fileUri, { + workspace, + baseline: getBaselinedErrorsForFile(this.fs, workspace.rootUri, fileUri), + }); } - writeDiagnosticsToBaselineFile(this.fs, rootUri, filesWithDiagnostics, true); }; protected async onExecuteCommand( @@ -1261,20 +1261,55 @@ export abstract class LanguageServerBase implements LanguageServerInterface, Dis return rule; } - protected onAnalysisCompletedHandler(fs: FileSystem, results: AnalysisResults): void { + protected async onAnalysisCompletedHandler(fs: FileSystem, results: AnalysisResults): Promise { // Send the computed diagnostics to the client. results.diagnostics.forEach((fileDiag) => { if (!this.canNavigateToFile(fileDiag.fileUri, fs)) { return; } - this.sendDiagnostics(fs, fileDiag); + this.sendDiagnostics(fs, { ...fileDiag, reason: results.reason }); }); results.configParseErrors.forEach((error) => this.connection.sendNotification(ShowMessageNotification.type, { message: error, type: MessageType.Error }) ); + // if any baselined diagnostics disappeared, update the baseline for the effected files + if (results.reason === 'analysis') { + const filesRequiringBaselineUpdate = new Map(); + for (const [fileUri, savedFileInfo] of this.savedFilesForBaselineUpdate.entries()) { + // can't use result.diagnostics because we need the diagnostics from the previous analysis since + // saves don't trigger checking (i think) + const fileDiagnostics = this.documentsWithDiagnostics[fileUri.toString()]; + if (!fileDiagnostics || fileDiagnostics.reason !== 'analysis') { + continue; + } + const sourceFile = savedFileInfo.workspace.service.getSourceFile(fileUri); + if (sourceFile && !sourceFile.isCheckingRequired()) { + const baselineInfo = getBaselinedErrorsForFile(this.fs, savedFileInfo.workspace.rootUri!, fileUri); + if ( + // no baseline file exists or no baselined errors exist for this file + !baselineInfo.length || + // there are diagnostics that haven't been baselined, so we don't want to write them + // because the user will have to either fix the diagnostics or explicitly write them to the + // baseline themselves + fileDiagnostics.diagnostics.some((diagnostic) => !diagnostic.baselineStatus) + ) { + continue; + } + if (!filesRequiringBaselineUpdate.has(savedFileInfo.workspace)) { + filesRequiringBaselineUpdate.set(savedFileInfo.workspace, []); + } + filesRequiringBaselineUpdate.get(savedFileInfo.workspace)!.push(fileDiagnostics); + } + } + for (const [workspace, files] of filesRequiringBaselineUpdate.entries()) { + writeDiagnosticsToBaselineFile(this.fs, workspace.rootUri!, files, true); + } + this.savedFilesForBaselineUpdate.clear(); + } + if (!this._progressReporter.isEnabled(results)) { // Make sure to disable progress bar if it is currently active. // This can happen if a user changes typeCheckingMode in the middle @@ -1332,6 +1367,7 @@ export abstract class LanguageServerBase implements LanguageServerInterface, Dis fileUri: fileWithDiagnostics.fileUri, diagnostics: [], version: undefined, + reason: 'tracking', }); } } @@ -1396,13 +1432,12 @@ export abstract class LanguageServerBase implements LanguageServerInterface, Dis }; } - protected async sendDiagnostics(fs: FileSystem, fileWithDiagnostics: FileDiagnostics) { + protected async sendDiagnostics( + fs: FileSystem, + fileWithDiagnostics: FileDiagnostics & { reason: 'analysis' | 'tracking' } + ) { const key = fileWithDiagnostics.fileUri.toString(); - if (fileWithDiagnostics.diagnostics.length === 0) { - delete this.documentsWithDiagnostics[key]; - } else { - this.documentsWithDiagnostics[key] = fileWithDiagnostics; - } + this.documentsWithDiagnostics[key] = fileWithDiagnostics; this.connection.sendDiagnostics(await this.convertDiagnostics(fs, fileWithDiagnostics)); } From 44ca5267dec2b435dc14aa7a5756ffcad3a955a6 Mon Sep 17 00:00:00 2001 From: detachhead Date: Wed, 25 Sep 2024 23:52:36 +1000 Subject: [PATCH 28/28] document baseline --- README.md | 56 +++++++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 42 insertions(+), 14 deletions(-) diff --git a/README.md b/README.md index 4bf4f3d5e..e7909d20f 100644 --- a/README.md +++ b/README.md @@ -270,6 +270,44 @@ when renaming a package or module, basedpyright will update all usages to the ne ![](https://github.com/user-attachments/assets/6207fe90-027a-4227-a1ed-d2c4406ad38c) +### baseline (beta) + +have you ever wanted to adopt a new tool or enable new checks in an existing project, only to be immediately bombarded with thousands of errors you'd have to fix? baseline solves this problem by allowing you to only report errors on new or modified code. it works by generating a baseline file keeping track of the existing errors in your project so that only errors in newly eritten or modified code get reported. + +to enable baseline, run `basedpyright --writebaseline` in your terminal or run the _"basedpyright: Write new errors to baseline"_ task in vscode. this will generate a `./basedpyright/baseline.json` for your project. you should commit this file so others working on your project can benefit from it too. + +this file gets automatically updated as errors are removed over time in both the CLI and the language server. if you ever need to baseline new errors or an error that resurfaced because you've modified the same line of code it was on, just run that command again. + +#### how does it work? + +each baselined error is stored and matched by the following details: + +- the path of the file it's in (relative to the project root) +- its diagnostic rule name (eg. `reportGeneralTypeIssues`) +- the position of the error in the file (column only, which prevents errors from resurfacing when you add or remove lines in a file) + +no baseline matching strategy is perfect, so this is subject to change. baseline is in beta so if you have any feedback please [raise an issue](https://github.com/DetachHead/basedpyright/issues/new/choose). + +#### how is this different to `# pyright: ignore` comments? + +ignore comments are typically used to suppress a false positive or workaround some limitation in the type checker. baselining is a way to suppress many valid instances of an error across your whole project, to avoid the burden of having to update thousands of lines of old code just to adopt stricter checks on your new code. + +#### credit + +this is heavily inspired by [basedmypy](https://kotlinisland.github.io/basedmypy/baseline). + +### better defaults + +we believe that type checkers and linters should be as strict as possible by default, making the user aware of all the available rules so they can more easily make informed decisions about which rules they don't want enabled in their project. that's why the following defaults have been changed in basedpyright + +#### `typeCheckingMode` + +used to be `basic`, but now defaults to `all`. while this may seem daunting at first, we support [baselining](#baseline-beta) to allow for easy adoption of more strict rules in existing codebases. + +#### `pythonPlatform` + +used to assume that the operating system pyright is being run on is the only operating system your code will run on, which is rarely the case. in basedpyright, `pythonPlatform` defaults to `All`, which assumes your code can run on any operating system. + ### errors on invalid configuration in pyright, if you have any invalid config, it may or may not print a warning to the console, then it will continue type-checking and the exit code will be 0 as long as there were no type errors: @@ -302,18 +340,6 @@ from baz import foo as baz, bar as baz # no error basedpyright solves both of these problems by always reporting an error on a redeclaration or an import with the same name as an existing import. -### better defaults - -we believe that type checkers and linters should be as strict as possible by default, making the user aware of all the available rules so they can more easily make informed decisions about which rules they don't want enabled in their project. that's why the following defaults have been changed in basedpyright - -#### `typeCheckingMode` - -used to be `basic`, but now defaults to `all`. in the future we intend to add [baseline](https://kotlinisland.github.io/basedmypy/baseline.html) to allow for easy adoption of more strict rules in existing codebases. - -#### `pythonPlatform` - -used to assume that the operating system pyright is being run on is the only operating system your code will run on, which is rarely the case. in basedpyright, `pythonPlatform` defaults to `All`, which assumes your code can run on any operating system. - ### inline `TypedDict` support pyright used to support defining `TypedDict`s inline, like so: @@ -374,9 +400,11 @@ we accept translation fixes in basedpyright. [see the localization guidelines](h [basedmypy](https://github.com/kotlinisland/basedmypy) is a fork of mypy with a similar goal in mind: to fix some of the serious problems in mypy that do not seem to be a priority for the maintainers. it also adds many new features which may not be standardized but greatly improve the developer experience when working with python's far-from-perfect type system. -we aim to [port most of basedmypy's features to basedpyright](https://github.com/DetachHead/basedpyright/issues?q=is%3Aissue+is%3Aopen+label%3A%22basedmypy+feature+parity%22), however as mentioned above our priority is to first fix the critical problems with pyright. +while the two projects have similar goals, there are some differences: -note that any non-standard features we add will be optional, as we intend to support library developers who can't control what type checker their library is used with. +- basedmypy makes breaking changes to improve the typing system and its syntax. for example, it supports intersections, `(int) -> str` function type syntax and `foo is int` syntax for type guards. [more info here](https://kotlinisland.github.io/basedmypy/based_features.html) +- basedpyright intends to be fully backwards compatible with all standard typing functionality. non-standard features will be fully optional and can be disabled, as we intend to support library developers who can't control what type checker their library is used with. +- basedpyright's two main goals are to improve the type checker's accuracy and reliability with existing syntax, and to bridge the gap between pylance and pyright # pypi package