From de76209a985d94e21e613460a76bb09886cf082d Mon Sep 17 00:00:00 2001 From: detachhead Date: Wed, 2 Oct 2024 00:14:37 +1000 Subject: [PATCH] refactor baseline code into a class, move some duplicated logic from both the language server and cli to the class and fix some bugs with the automatic baseline updating --- .../src/analyzer/sourceFile.ts | 7 +- packages/pyright-internal/src/baseline.ts | 399 ++++++++++-------- .../src/commands/writeBaseline.ts | 20 +- .../pyright-internal/src/common/diagnostic.ts | 5 + .../src/languageServerBase.ts | 48 +-- packages/pyright-internal/src/pyright.ts | 32 +- 6 files changed, 274 insertions(+), 237 deletions(-) diff --git a/packages/pyright-internal/src/analyzer/sourceFile.ts b/packages/pyright-internal/src/analyzer/sourceFile.ts index 772de617e..ec4f22257 100644 --- a/packages/pyright-internal/src/analyzer/sourceFile.ts +++ b/packages/pyright-internal/src/analyzer/sourceFile.ts @@ -48,7 +48,7 @@ import { SourceMapper } from './sourceMapper'; import { SymbolTable } from './symbol'; import { TestWalker } from './testWalker'; import { TypeEvaluator } from './typeEvaluatorTypes'; -import { sortDiagnosticsAndMatchBaseline } from '../baseline'; +import { BaselineHandler } from '../baseline'; // Limit the number of import cycles tracked per source file. const _maxImportCyclesPerFile = 4; @@ -1247,7 +1247,10 @@ export class SourceFile { // Now add in the "unnecessary type ignore" diagnostics. diagList = diagList.concat(unnecessaryTypeIgnoreDiags); - diagList = sortDiagnosticsAndMatchBaseline(this.fileSystem, configOptions.projectRoot, this._uri, diagList); + diagList = new BaselineHandler(this.fileSystem, configOptions.projectRoot).sortDiagnosticsAndMatchBaseline( + 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 753b6b04d..c5ac21eb1 100644 --- a/packages/pyright-internal/src/baseline.ts +++ b/packages/pyright-internal/src/baseline.ts @@ -1,14 +1,15 @@ import { DiagnosticRule } from './common/diagnosticRules'; import { FileDiagnostics } from './common/diagnosticSink'; import { Uri } from './common/uri/uri'; -import { compareDiagnostics, convertLevelToCategory, Diagnostic, DiagnosticCategory } from './common/diagnostic'; +import { compareDiagnostics, convertLevelToCategory, Diagnostic, isHintDiagnostic } from './common/diagnostic'; import { extraOptionDiagnosticRules } from './common/configOptions'; import { fileExists } from './common/uri/uriUtils'; -import { FileSystem, ReadOnlyFileSystem } from './common/fileSystem'; +import { FileSystem } from './common/fileSystem'; import { pluralize } from './common/stringUtils'; import { diffArrays } from 'diff'; import { assert } from './common/debug'; import { Range } from './common/textRange'; +import { add } from 'lodash'; export interface BaselinedDiagnostic { code: DiagnosticRule | undefined; @@ -24,185 +25,253 @@ export interface BaselinedDiagnostic { }; } -interface BaselineFile { +/** + * the JSON structure of the baseline file + */ +interface BaselineData { files: { [filePath: string]: BaselinedDiagnostic[]; }; } -const lineCount = (range: Range) => range.end.line - range.start.line + 1; +type OptionalIfFalse = T | (Bool extends true ? never : undefined); -export const baselineFilePath = (rootDir: Uri) => rootDir.combinePaths('.basedpyright/baseline.json'); +/** + * details about the difference between the previous version and the current version of the baseline file + */ +class BaselineDiff { + readonly baselinedErrorCount: number; + readonly newErrorCount: number; + readonly diff: number; -const diagnosticsToBaseline = (rootDir: Uri, filesWithDiagnostics: readonly FileDiagnostics[]): BaselineFile => { - const baselineData: BaselineFile = { - files: {}, - }; - for (const fileWithDiagnostics of filesWithDiagnostics) { - const filePath = rootDir.getRelativePath(fileWithDiagnostics.fileUri)!.toString(); - const errorDiagnostics = fileWithDiagnostics.diagnostics.filter( - (diagnostic) => - ![ - DiagnosticCategory.Deprecated, - DiagnosticCategory.UnreachableCode, - DiagnosticCategory.UnusedCode, - ].includes(diagnostic.category) || diagnostic.baselineStatus === 'baselined with hint' - ); - 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, - range: { - startColumn: diagnostic.range.start.character, - endColumn: diagnostic.range.end.character, - lineCount: lineCount(diagnostic.range), - }, - })) - ); + constructor( + private _rootDir: Uri, + readonly previousBaseline: BaselineData, + readonly newBaseline: BaselineData, + private readonly _forced: T + ) { + this.baselinedErrorCount = Object.values(previousBaseline.files).flatMap((file) => file).length; + this.newErrorCount = Object.values(newBaseline.files).flatMap((file) => file).length; + this.diff = this.newErrorCount - this.baselinedErrorCount; } - return baselineData; -}; -/** - * @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 = ( - fs: FileSystem, - rootDir: Uri, - filesWithDiagnostics: readonly FileDiagnostics[], - openFilesOnly: boolean -): 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]; + getSummaryMessage = (): OptionalIfFalse => { + let message = ''; + if (this.diff === 0) { + if (!this._forced) { + // if the error count didn't change and the baseline update was not explicitly requested by the user, + // that means nothing changed so don't show any message + return undefined as OptionalIfFalse; + } + message += "error count didn't change"; + } else if (this.diff > 0) { + message += `went up by ${this.diff}`; + } else { + message += `went down by ${this.diff * -1}`; } + + return `updated ${this._rootDir.getRelativePath(baselineFilePath(this._rootDir))} with ${pluralize( + this.newErrorCount, + 'error' + )} (${message})`; + }; +} + +export class BaselineHandler { + readonly fileUri: Uri; + + /** + * when {@link baselineData} is `undefined` that means there is currently no baseline file, in which case + * none of this functionality should be observed by the user until they explicitly opt in to the baseline + * feature + */ + constructor(private _fs: FileSystem, private _rootDir: Uri) { + this.fileUri = baselineFilePath(_rootDir); } - 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]; + + getContents = (): BaselineData | undefined => { + let baselineFileContents: string | undefined; + try { + baselineFileContents = this._fs.readFileSync(this.fileUri, 'utf8'); + } catch (e) { + // assume the file didn't exist + return undefined; } - } - const baselineFile = baselineFilePath(rootDir); - fs.mkdirSync(baselineFile.getDirectory(), { recursive: true }); - fs.writeFileSync(baselineFile, JSON.stringify(result, undefined, 4), null); - return result; -}; - -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 JSON.parse(baselineFileContents); + }; - 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 = fs.readFileSync(path, 'utf8'); - } catch (e) { - return { files: {} }; - } - return JSON.parse(baselineFileContents); -}; - -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(fs, rootDir).files[rootDir.getRelativePath(file)!.toString()]; - } - return result ?? []; -}; - -export const sortDiagnosticsAndMatchBaseline = ( - fs: ReadOnlyFileSystem, - rootDir: Uri, - file: Uri, - diagnostics: Diagnostic[] -): Diagnostic[] => { - diagnostics.sort(compareDiagnostics); - const diff = diffArrays(getBaselinedErrorsForFile(fs, rootDir, file), diagnostics, { - comparator: (baselinedDiagnostic, diagnostic) => - baselinedDiagnostic.code === diagnostic.getRule() && - baselinedDiagnostic.range.startColumn === diagnostic.range.start.character && - baselinedDiagnostic.range.endColumn === diagnostic.range.end.character && - //for backwards compatibility with old baseline files, only check this if it's present - (baselinedDiagnostic.range.lineCount === undefined || - baselinedDiagnostic.range.lineCount === lineCount(diagnostic.range)), - }); - const result = []; - for (const change of diff) { - if (change.removed) { - continue; + /** + * updates the baseline file + * + * @param force whether to write the baseline file even if there are new errors or if there is not baseline + * file yet + * @param removeDeletedFiles whether to check whether each file listed in the baseline still exists, and + * delete its errors from the baseline file if not. this option mainly exists for performance reasons (but + * i haven't actually checked whether it has a noticable impact) + * @param filesWithDiagnostics the new diagnostics to write to the baseline file + */ + write = ( + force: T, + removeDeletedFiles: boolean, + filesWithDiagnostics: readonly FileDiagnostics[] + ): OptionalIfFalse> => { + type Result = OptionalIfFalse>; + const baselineData = this.getContents(); + if (!force) { + if (!baselineData) { + // there currently is no baseline file and the user did not explicitly ask for one, so we do nothing + return undefined as Result; + } + /** diagnostics that haven't yet been baselined */ + const newDiagnostics = filesWithDiagnostics.map((file) => ({ + ...file, + diagnostics: file.diagnostics.filter( + (diagnostic) => !diagnostic.baselineStatus && !isHintDiagnostic(diagnostic) + ), + })); + if (newDiagnostics.map((fileWithDiagnostics) => fileWithDiagnostics.diagnostics.length).reduce(add, 0)) { + // there are unbaselined diagnostics and the user did not explicitly ask to update the baseline, so we do + // nothing + return undefined as Result; + } } - if (change.added) { - assert(change.value[0] instanceof Diagnostic, "change object wasn't a Diagnostic"); - result.push(...(change.value as Diagnostic[])); - } else { - // if not added and not removed - // 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? - for (const diagnostic of change.value) { - assert( - diagnostic instanceof Diagnostic, - 'diff thingy returned the old value instead of the new one???' - ); - let newDiagnostic; - const diagnosticRule = diagnostic.getRule() as DiagnosticRule | undefined; - if (diagnosticRule) { - for (const { name, get } of extraOptionDiagnosticRules) { - if (get().includes(diagnosticRule)) { - newDiagnostic = diagnostic.copy({ - category: convertLevelToCategory(name), - baselineStatus: 'baselined with hint', - }); - newDiagnostic.setRule(diagnosticRule); - // none of these rules should have multiple extra diagnostic levels so we break after the first match - break; + const newBaselineFiles = this._filteredDiagnosticsToBaselineFormat(filesWithDiagnostics).files; + const previousBaselineFiles = baselineData?.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 previousBaselineFiles) { + if ( + !newBaselineFiles[filePath] && + (!removeDeletedFiles || fileExists(this._fs, this._rootDir.combinePaths(filePath))) + ) { + newBaselineFiles[filePath] = previousBaselineFiles[filePath]; + } + } + const result: BaselineData = { 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(newBaselineFiles).sort()) { + // remove files where there are no errors + if (newBaselineFiles[file].length) { + result.files[file] = newBaselineFiles[file]; + } + } + this._fs.mkdirSync(this.fileUri.getDirectory(), { recursive: true }); + this._fs.writeFileSync(this.fileUri, JSON.stringify(result, undefined, 4), null); + return new BaselineDiff(this._rootDir, { files: previousBaselineFiles }, result, force); + }; + + sortDiagnosticsAndMatchBaseline = (moduleUri: Uri, diagnostics: Diagnostic[]): Diagnostic[] => { + diagnostics.sort(compareDiagnostics); + const baselinedErrorsForFile = this._getBaselinedErrorsForFile(moduleUri); + if (!baselinedErrorsForFile) { + return diagnostics; + } + const diff = diffArrays(baselinedErrorsForFile, diagnostics, { + comparator: (baselinedDiagnostic, diagnostic) => + baselinedDiagnostic.code === diagnostic.getRule() && + baselinedDiagnostic.range.startColumn === diagnostic.range.start.character && + baselinedDiagnostic.range.endColumn === diagnostic.range.end.character && + //for backwards compatibility with old baseline files, only check this if it's present + (baselinedDiagnostic.range.lineCount === undefined || + baselinedDiagnostic.range.lineCount === lineCount(diagnostic.range)), + }); + const result = []; + for (const change of diff) { + if (change.removed) { + continue; + } + if (change.added) { + assert(change.value[0] instanceof Diagnostic, "change object wasn't a Diagnostic"); + result.push(...(change.value as Diagnostic[])); + } else { + // if not added and not removed + // 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? + for (const diagnostic of change.value) { + assert( + diagnostic instanceof Diagnostic, + 'diff thingy returned the old value instead of the new one???' + ); + let newDiagnostic; + const diagnosticRule = diagnostic.getRule() as DiagnosticRule | undefined; + if (diagnosticRule) { + for (const { name, get } of extraOptionDiagnosticRules) { + if (get().includes(diagnosticRule)) { + newDiagnostic = diagnostic.copy({ + category: convertLevelToCategory(name), + baselineStatus: 'baselined with hint', + }); + newDiagnostic.setRule(diagnosticRule); + // none of these rules should have multiple extra diagnostic levels so we break after the first match + break; + } } } + if (!newDiagnostic) { + newDiagnostic = diagnostic.copy({ baselineStatus: 'baselined' }); + } + result.push(newDiagnostic); } - if (!newDiagnostic) { - newDiagnostic = diagnostic.copy({ baselineStatus: 'baselined' }); - } - result.push(newDiagnostic); } } - } - return result; -}; + return result; + }; + + /** + * filters out diagnostics that are baselined, but keeps any that have been turned into hints. so you will need + * to filter it further using {@link isHintDiagnostic} if you want those removed as well + */ + filterOutBaselinedDiagnostics = (filesWithDiagnostics: readonly FileDiagnostics[]): readonly FileDiagnostics[] => + filesWithDiagnostics.map((file) => ({ + ...file, + diagnostics: file.diagnostics.filter((diagnostic) => diagnostic.baselineStatus !== 'baselined'), + })); + + private _getBaselinedErrorsForFile = (file: Uri): BaselinedDiagnostic[] => { + const relativePath = this._rootDir.getRelativePath(file); + let result; + // if this is undefined it means the file isn't in the workspace + if (relativePath) { + result = this.getContents()?.files[relativePath.toString()]; + } + return result ?? []; + }; + + private _filteredDiagnosticsToBaselineFormat = (filesWithDiagnostics: readonly FileDiagnostics[]): BaselineData => { + const baselineData: BaselineData = { + files: {}, + }; + for (const fileWithDiagnostics of filesWithDiagnostics) { + const filePath = this._rootDir.getRelativePath(fileWithDiagnostics.fileUri)!.toString(); + const errorDiagnostics = fileWithDiagnostics.diagnostics.filter( + (diagnostic) => !isHintDiagnostic(diagnostic) || diagnostic.baselineStatus === 'baselined with hint' + ); + 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, + range: { + startColumn: diagnostic.range.start.character, + endColumn: diagnostic.range.end.character, + lineCount: lineCount(diagnostic.range), + }, + })) + ); + } + return baselineData; + }; +} + +const lineCount = (range: Range) => range.end.line - range.start.line + 1; + +export const baselineFilePath = (rootDir: Uri) => rootDir.combinePaths('.basedpyright/baseline.json'); diff --git a/packages/pyright-internal/src/commands/writeBaseline.ts b/packages/pyright-internal/src/commands/writeBaseline.ts index d27a08f8a..16cf40a95 100644 --- a/packages/pyright-internal/src/commands/writeBaseline.ts +++ b/packages/pyright-internal/src/commands/writeBaseline.ts @@ -1,10 +1,5 @@ import { ServerCommand } from './commandController'; -import { - baselineFilePath, - getBaselinedErrors, - getBaselineSummaryMessage, - writeDiagnosticsToBaselineFile, -} from '../baseline'; +import { baselineFilePath, BaselineHandler } from '../baseline'; import { LanguageServerInterface } from '../common/languageServerInterface'; import { matchFileSpecs } from '../common/configOptions'; import { Uri } from '../common/uri/uri'; @@ -36,7 +31,7 @@ export class WriteBaselineCommand implements ServerCommand { if (workspace) { const workspaceRoot = workspace.rootUri; if (workspaceRoot) { - const previousBaseline = getBaselinedErrors(workspace.service.fs, workspaceRoot); + const baselineHandler = new BaselineHandler(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 @@ -49,16 +44,9 @@ export class WriteBaselineCommand implements ServerCommand { matchFileSpecs(configOptions, Uri.file(filePath, this._ls.serviceProvider)) ) .map(([_, diagnostics]) => diagnostics); - const newBaseline = writeDiagnosticsToBaselineFile( - workspace.service.fs, - workspaceRoot, - filteredFiles, - true - ); + const newBaseline = baselineHandler.write(true, true, filteredFiles); workspace.service.baselineUpdated(); - this._ls.window.showInformationMessage( - getBaselineSummaryMessage(workspaceRoot, previousBaseline, newBaseline) - ); + this._ls.window.showInformationMessage(newBaseline.getSummaryMessage()); return; } } diff --git a/packages/pyright-internal/src/common/diagnostic.ts b/packages/pyright-internal/src/common/diagnostic.ts index 49fb9fd58..73249733e 100644 --- a/packages/pyright-internal/src/common/diagnostic.ts +++ b/packages/pyright-internal/src/common/diagnostic.ts @@ -69,6 +69,11 @@ export function convertLevelToCategory(level: LspDiagnosticLevel) { } } +export const isHintDiagnostic = (diagnostic: Diagnostic) => + diagnostic.category === DiagnosticCategory.UnusedCode || + diagnostic.category === DiagnosticCategory.UnreachableCode || + diagnostic.category === DiagnosticCategory.Deprecated; + export interface DiagnosticAction { action: string; } diff --git a/packages/pyright-internal/src/languageServerBase.ts b/packages/pyright-internal/src/languageServerBase.ts index 76f0a6956..ab9354289 100644 --- a/packages/pyright-internal/src/languageServerBase.ts +++ b/packages/pyright-internal/src/languageServerBase.ts @@ -139,7 +139,7 @@ import { InitStatus, WellKnownWorkspaceKinds, Workspace, WorkspaceFactory } from import { website } from './constants'; import { SemanticTokensProvider, SemanticTokensProviderLegend } from './languageService/semanticTokensProvider'; import { RenameUsageFinder } from './analyzer/renameUsageFinder'; -import { BaselinedDiagnostic, getBaselinedErrorsForFile, writeDiagnosticsToBaselineFile } from './baseline'; +import { BaselineHandler } from './baseline'; export abstract class LanguageServerBase implements LanguageServerInterface, Disposable { // We support running only one "find all reference" at a time. @@ -186,10 +186,7 @@ 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[] } - >(); + protected readonly savedFilesForBaselineUpdate = new Set(); // The URIs for which diagnostics are reported readonly documentsWithDiagnostics: Record = {}; @@ -1170,14 +1167,7 @@ export abstract class LanguageServerBase implements LanguageServerInterface, Dis * 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); - if (workspace.rootUri) { - this.savedFilesForBaselineUpdate.set(fileUri, { - workspace, - baseline: getBaselinedErrorsForFile(this.fs, workspace.rootUri, fileUri), - }); - } + this.savedFilesForBaselineUpdate.add(params.textDocument.uri); }; protected async onExecuteCommand( @@ -1284,31 +1274,31 @@ export abstract class LanguageServerBase implements LanguageServerInterface, Dis !results.requiringAnalysisCount.cells ) { const filesRequiringBaselineUpdate = new Map(); - for (const [fileUri, savedFileInfo] of this.savedFilesForBaselineUpdate.entries()) { + for (const textDocumentUri of this.savedFilesForBaselineUpdate) { + const fileUri = Uri.file(textDocumentUri, this.serviceProvider); // 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 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; + const workspace = await this.getWorkspaceForFile(fileUri); + if (!filesRequiringBaselineUpdate.has(workspace)) { + filesRequiringBaselineUpdate.set(workspace, []); } - if (!filesRequiringBaselineUpdate.has(savedFileInfo.workspace)) { - filesRequiringBaselineUpdate.set(savedFileInfo.workspace, []); - } - filesRequiringBaselineUpdate.get(savedFileInfo.workspace)!.push(fileDiagnostics); + filesRequiringBaselineUpdate.get(workspace)!.push(fileDiagnostics); } for (const [workspace, files] of filesRequiringBaselineUpdate.entries()) { - writeDiagnosticsToBaselineFile(this.fs, workspace.rootUri!, files, true); + if (!workspace.rootUri) { + continue; + } + const baseline = new BaselineHandler(this.fs, workspace.rootUri); + const baselineDiffSummary = baseline.write(false, false, files)?.getSummaryMessage(); + if (baselineDiffSummary) { + this.console.info( + `${baselineDiffSummary}. files: ${files.map((file) => file.fileUri.toString()).join(', ')}` + ); + } } this.savedFilesForBaselineUpdate.clear(); } diff --git a/packages/pyright-internal/src/pyright.ts b/packages/pyright-internal/src/pyright.ts index 981f433b6..f1e6980ec 100644 --- a/packages/pyright-internal/src/pyright.ts +++ b/packages/pyright-internal/src/pyright.ts @@ -30,7 +30,7 @@ import { CommandLineOptions as PyrightCommandLineOptions } from './common/comman import { ConsoleInterface, LogLevel, StandardConsole, StderrConsole } from './common/console'; import { fail } from './common/debug'; import { createDeferred } from './common/deferred'; -import { Diagnostic, DiagnosticCategory } from './common/diagnostic'; +import { Diagnostic, DiagnosticCategory, isHintDiagnostic } from './common/diagnostic'; import { FileDiagnostics } from './common/diagnosticSink'; import { FullAccessHost } from './common/fullAccessHost'; import { combinePaths, normalizePath } from './common/pathUtils'; @@ -48,8 +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 { getBaselinedErrors, getBaselineSummaryMessage, writeDiagnosticsToBaselineFile } from './baseline'; -import { add } from 'lodash'; +import { BaselineHandler } from './baseline'; import { pluralize } from './common/stringUtils'; type SeverityLevel = 'error' | 'warning' | 'information'; @@ -463,24 +462,12 @@ const outputResults = ( ? Uri.file(options.executionRoot ?? '', service.serviceProvider) : options.executionRoot; - 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, 0) - ) { - const previousBaseline = getBaselinedErrors(service.fs, rootDir); - // don't write the baseline file if there wasn't one there already unless the user explicitly asked for it - if (args.writebaseline || Object.keys(previousBaseline.files).length) { - const newBaseline = writeDiagnosticsToBaselineFile(service.fs, rootDir, results.diagnostics, false); - console.info(getBaselineSummaryMessage(rootDir, previousBaseline, newBaseline)); - } + const baselineFile = new BaselineHandler(service.fs, rootDir); + const baselineDiffMessage = baselineFile.write(args.writebaseline, true, results.diagnostics)?.getSummaryMessage(); + if (baselineDiffMessage) { + console.info(baselineDiffMessage); } + const filteredDiagnostics = baselineFile.filterOutBaselinedDiagnostics(results.diagnostics); const treatWarningsAsErrors = !!args.warnings; let errorCount = 0; @@ -1296,11 +1283,6 @@ const printDiagnosticSummary = (result: DiagnosticResult) => { ); }; -const isHintDiagnostic = (diagnostic: Diagnostic) => - diagnostic.category === DiagnosticCategory.UnusedCode || - diagnostic.category === DiagnosticCategory.UnreachableCode || - diagnostic.category === DiagnosticCategory.Deprecated; - function reportDiagnosticsAsText( fileDiagnostics: readonly FileDiagnostics[], minSeverityLevel: SeverityLevel