Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

baseline #608

Merged
merged 28 commits into from
Sep 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
60fdb69
add `--writebaseline` cli arg
DetachHead Aug 22, 2024
4924e60
baseline matching
DetachHead Aug 24, 2024
839b63a
support baseline in the language server & vscode extension
DetachHead Aug 25, 2024
100f4b0
fix new errors being automatically written to the baseline
DetachHead Aug 27, 2024
0bcf17e
don't remove baselined errors from unopened files when running the wr…
DetachHead Aug 27, 2024
14d8712
don't rely on the message for baseline matching because they change o…
DetachHead Aug 29, 2024
6c0b788
don't baaseline unreachable/unused/deprecated diagnostic levels as th…
DetachHead Aug 29, 2024
366ebca
report baselined errors that can be represented with a hint diagnosti…
DetachHead Aug 29, 2024
2e9332a
output baseline info in the cli
DetachHead Sep 2, 2024
33f7cb5
fix incorrect baseline error counts
DetachHead Sep 3, 2024
7a5ab46
fix python tests not being typechecked
DetachHead Sep 3, 2024
3443af9
fix crash when reading/writing baseline when no workspace is open and…
DetachHead Sep 11, 2024
9d223ae
fix vscode jest
DetachHead Sep 15, 2024
82e0b92
refactor baseline stuff to make it easier to write tests for
DetachHead Sep 15, 2024
fa8ad63
fix baseline test
DetachHead Sep 16, 2024
7a2da0a
fix baseline being automatically rewritten when error count goes down…
DetachHead Sep 17, 2024
d90adb9
don't store columns in baseline because they aren't used for matching
DetachHead Sep 19, 2024
53ca1e7
rename vscode command category from pyright to basedpyright
DetachHead Sep 20, 2024
e292113
remove duplicated code from `runSingleThreaded` and `runMultiThreaded…
DetachHead Sep 21, 2024
3d968e2
use exclude to prevent basedpyright self check from checking ignored …
DetachHead Sep 21, 2024
fe62955
refactor baseline stuff to use pyright filesystem, move summary messa…
DetachHead Sep 22, 2024
8bf5b76
address potential concerns that this change doesnt have enough test c…
DetachHead Sep 22, 2024
188981f
fix cli crash when checking baseline file if there are no files
DetachHead Sep 22, 2024
a14539d
sort baselined files before writing them to the baseline file to prev…
DetachHead Sep 22, 2024
75028ed
prevent excluded files from getting baselined when using the write ba…
DetachHead Sep 22, 2024
14d8bc0
fix diagnostics not being deleted when workspace removed i think
DetachHead Sep 24, 2024
58a9f55
fix baselined errors being incorrectly deleted if a file is saved bef…
DetachHead Sep 24, 2024
44ca526
document baseline
DetachHead Sep 25, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .vscode/settings.json
Original file line number Diff line number Diff line change
Expand Up @@ -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 --"
}
56 changes: 42 additions & 14 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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

Expand Down
1 change: 1 addition & 0 deletions docs/command-line.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ Pyright can be run as either a VS Code extension or as a command-line tool. The
| --level <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 `<FILE OR DIRECTORY>` | Use the configuration file at this location |
| --pythonpath `<FILE>` | Path to the Python interpreter (2) |
| --pythonplatform `<PLATFORM>` | Analyze for platform (Darwin, Linux, Windows) |
Expand Down
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
4 changes: 2 additions & 2 deletions packages/pyright-internal/src/analyzer/analysis.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion packages/pyright-internal/src/analyzer/program.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand Down
5 changes: 5 additions & 0 deletions packages/pyright-internal/src/analyzer/service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
3 changes: 3 additions & 0 deletions packages/pyright-internal/src/analyzer/sourceFile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -1246,6 +1247,8 @@ export class SourceFile {
// Now add in the "unnecessary type ignore" diagnostics.
diagList = diagList.concat(unnecessaryTypeIgnoreDiags);

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
// and deprecated diagnostics.
Expand Down
176 changes: 176 additions & 0 deletions packages/pyright-internal/src/baseline.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,176 @@
import { DiagnosticRule } from './common/diagnosticRules';
import { FileDiagnostics } from './common/diagnosticSink';
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';

export interface BaselinedDiagnostic {
code: DiagnosticRule | undefined;
range: { startColumn: number; endColumn: number };
}

interface BaselineFile {
files: {
[filePath: string]: BaselinedDiagnostic[];
};
}

export const baselineFilePath = (rootDir: Uri) => rootDir.combinePaths('.basedpyright/baseline.json');

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 },
}))
);
}
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];
}
}
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 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 `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 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;
const matchedIndex = baselinedErrorsForFile.findIndex(
(baselinedError) =>
baselinedError.code === diagnosticRule &&
baselinedError.range.startColumn === diagnostic.range.start.character &&
baselinedError.range.endColumn === 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;
}
}
}
diagnostic.baselineStatus = 'baselined';
}
}
};
6 changes: 6 additions & 0 deletions packages/pyright-internal/src/commands/commandController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<any>;
Expand All @@ -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<any> {
Expand All @@ -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<string>(1, 'Unsupported command');
}
Expand Down
Loading
Loading