Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 6 additions & 0 deletions docs/benefits-over-pyright/new-diagnostic-rules.md
Original file line number Diff line number Diff line change
Expand Up @@ -276,3 +276,9 @@ _ = Foo() # no error
this is allegedly for [performance reasons](https://github.com/microsoft/pyright/issues/5026#issuecomment-1526479622), but basedpyright's `reportInvalidAbstractMethod` rule is reported on the method definition instead of the usage, so it doesn't have to check every method when instantiating every non-abstract class.

it also just makes more sense to report the error on the method definition anyway. methods decorated with `@abstractmethod` on classes that do not extend `ABC` will not raise a runtime error if they are instantiated, making them less safe.

## `reportPackageTypeVerificationError` / `reportPackageTypeVerificationWarning`

the pyright CLI's `--verifytypes` argument will run pyright with a completely separate set of diagnostics intended to validate that a package is correctly typed for distribution (ie. publishing on pypi). in pyright, there's no way to run these checks as part of the regular type checker, which means these diagnostics aren't visible in the language server.

basedpyright solves this by reporting these diagnostics under the `reportPackageTypeVerificationError` and `reportPackageTypeVerificationWarning` rules.
4 changes: 4 additions & 0 deletions docs/configuration/config-files.md
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,10 @@ The following settings allow more fine grained control over the **typeCheckingMo

- <a name="reportInvalidAbstractMethod"></a> **reportInvalidAbstractMethod** [boolean or string, optional]: Generate or suppress diagnostics for usages of `@abstractmethod` on a non-abstract class. [more info](../benefits-over-pyright/new-diagnostic-rules.md#reportinvalidabstractmethod)

- <a name="reportPackageTypeVerificationError"></a> **reportPackageTypeVerificationError** [boolean or string, optional]: Generate or suppress diagnostics for errors from the package type verifier. [more info](../benefits-over-pyright/new-diagnostic-rules.md#reportpackagetypeverificationerror--reportpackagetypeverificationwarning)

- <a name="reportPackageTypeVerificationWarning"></a> **reportPackageTypeVerificationWarning** [boolean or string, optional]: Generate or suppress diagnostics for warnings from the package type verifier. [more info](../benefits-over-pyright/new-diagnostic-rules.md#reportpackagetypeverificationerror--reportpackagetypeverificationwarning)

### Discouraged settings

there are rules in pyright that are discouraged in basedpyright because we provide a better alternative. these options are still available for backwards compatibility, but you shouldn't use them.
Expand Down
10 changes: 5 additions & 5 deletions packages/pyright-internal/src/analyzer/packageTypeVerifier.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
*/

import { CommandLineOptions } from '../common/commandLineOptions';
import { BasedConfigOptions, ConfigOptions, ExecutionEnvironment } from '../common/configOptions';
import { ConfigOptions, ExecutionEnvironment } from '../common/configOptions';
import { NullConsole } from '../common/console';
import { assert } from '../common/debug';
import { Diagnostic, DiagnosticAddendum, DiagnosticCategory } from '../common/diagnostic';
Expand Down Expand Up @@ -75,19 +75,20 @@ export class PackageTypeVerifier {
private _configOptions: ConfigOptions;
private _execEnv: ExecutionEnvironment;
private _importResolver: ImportResolver;
private _program: Program;

constructor(
private _serviceProvider: ServiceProvider,
private _host: Host,
commandLineOptions: CommandLineOptions,
private _packageName: string,
private _program: Program,
private _ignoreExternal = false
) {
const host = new FullAccessHost(_serviceProvider);
this._configOptions = new BasedConfigOptions(Uri.empty());
const console = new NullConsole();

this._configOptions = _program.configOptions;

// Make sure we have a default python platform and version.
// Allow the command-line parameters to override the normal defaults.
if (commandLineOptions.configSettings.pythonPlatform) {
Expand All @@ -107,8 +108,7 @@ export class PackageTypeVerifier {
}

this._execEnv = this._configOptions.findExecEnvironment(Uri.file('.', _serviceProvider));
this._importResolver = new ImportResolver(this._serviceProvider, this._configOptions, this._host);
this._program = new Program(this._importResolver, this._configOptions, this._serviceProvider);
this._importResolver = _program.importResolver;
}

verify(): PackageTypeReport {
Expand Down
2 changes: 2 additions & 0 deletions packages/pyright-internal/src/analyzer/program.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ import { getPrintTypeFlags } from './typePrinter';
import { TypeStubWriter } from './typeStubWriter';
import { Type } from './types';
import { BaselineHandler } from '../baseline';
import { PackageTypeVerifier } from './packageTypeVerifier';

const _maxImportDepth = 256;

Expand Down Expand Up @@ -139,6 +140,7 @@ export class Program {
private _editModeTracker = new EditModeTracker();
private _sourceFileFactory: ISourceFileFactory;

private _packageTypeVerifier?: PackageTypeVerifier;
baselineHandler: BaselineHandler;

constructor(
Expand Down
5 changes: 0 additions & 5 deletions packages/pyright-internal/src/analyzer/service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -983,11 +983,6 @@ export class AnalyzerService {
configOptions.venvPath = projectRoot.resolvePaths(languageServerOptions.venvPath);
}
}
if (languageServerOptions.baselineFile) {
if (!configOptions.baselineFile) {
configOptions.baselineFile = projectRoot.resolvePaths(languageServerOptions.baselineFile);
}
}
}

private _applyCommandLineOverrides(
Expand Down
3 changes: 0 additions & 3 deletions packages/pyright-internal/src/common/commandLineOptions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -156,9 +156,6 @@ export class CommandLineLanguageServerOptions {

// Virtual environments directory.
venvPath?: string | undefined;

//Path to baseline file.
baselineFile?: string | undefined;
}

// Some options can be specified from a source other than the pyright config file.
Expand Down
25 changes: 24 additions & 1 deletion packages/pyright-internal/src/common/configOptions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,11 @@ import { userFacingOptionsList } from './stringUtils';
// which is cringe because it doesn't change the exit code
/* eslint no-console: ["error", { allow: ["log"] }] */

interface VerifyTypes {
package: string;
ignoreExternal: boolean;
}

export enum PythonPlatform {
Darwin = 'Darwin',
Windows = 'Windows',
Expand Down Expand Up @@ -425,7 +430,9 @@ export interface DiagnosticRuleSet {
reportUnannotatedClassAttribute: DiagnosticLevel;
reportIncompatibleUnannotatedOverride: DiagnosticLevel;
reportInvalidAbstractMethod: DiagnosticLevel;
allowedUntypedLibraries: string[];
reportPackageTypeVerificationError: DiagnosticLevel;
reportPackageTypeVerificationWarning: DiagnosticLevel;
allowedUntypedLibraries: string[]; //TODO: this shouldnt be here because it's not a diagnostic rule
}

export function cloneDiagnosticRuleSet(diagSettings: DiagnosticRuleSet): DiagnosticRuleSet {
Expand Down Expand Up @@ -557,6 +564,8 @@ export function getDiagLevelDiagnosticRules() {
DiagnosticRule.reportUnannotatedClassAttribute,
DiagnosticRule.reportIncompatibleUnannotatedOverride,
DiagnosticRule.reportInvalidAbstractMethod,
DiagnosticRule.reportPackageTypeVerificationError,
DiagnosticRule.reportPackageTypeVerificationWarning,
];
}

Expand Down Expand Up @@ -694,6 +703,8 @@ export function getOffDiagnosticRuleSet(): DiagnosticRuleSet {
reportUnannotatedClassAttribute: 'none',
reportIncompatibleUnannotatedOverride: 'none',
reportInvalidAbstractMethod: 'none',
reportPackageTypeVerificationError: 'none',
reportPackageTypeVerificationWarning: 'none',
allowedUntypedLibraries: [],
};

Expand Down Expand Up @@ -813,6 +824,8 @@ export function getBasicDiagnosticRuleSet(): DiagnosticRuleSet {
reportUnannotatedClassAttribute: 'none',
reportIncompatibleUnannotatedOverride: 'none',
reportInvalidAbstractMethod: 'none',
reportPackageTypeVerificationError: 'none',
reportPackageTypeVerificationWarning: 'none',
allowedUntypedLibraries: [],
};

Expand Down Expand Up @@ -932,6 +945,8 @@ export function getStandardDiagnosticRuleSet(): DiagnosticRuleSet {
reportUnannotatedClassAttribute: 'none',
reportIncompatibleUnannotatedOverride: 'none',
reportInvalidAbstractMethod: 'none',
reportPackageTypeVerificationError: 'none',
reportPackageTypeVerificationWarning: 'none',
allowedUntypedLibraries: [],
};

Expand Down Expand Up @@ -1050,6 +1065,8 @@ export const getRecommendedDiagnosticRuleSet = (): DiagnosticRuleSet => ({
reportUnannotatedClassAttribute: 'warning',
reportIncompatibleUnannotatedOverride: 'none', // TODO: change to error when we're confident there's no performance issues with this rule
reportInvalidAbstractMethod: 'warning',
reportPackageTypeVerificationError: 'error',
reportPackageTypeVerificationWarning: 'warning',
allowedUntypedLibraries: [],
});

Expand Down Expand Up @@ -1165,6 +1182,8 @@ export const getAllDiagnosticRuleSet = (): DiagnosticRuleSet => ({
reportUnannotatedClassAttribute: 'error',
reportIncompatibleUnannotatedOverride: 'error',
reportInvalidAbstractMethod: 'error',
reportPackageTypeVerificationError: 'error',
reportPackageTypeVerificationWarning: 'error',
allowedUntypedLibraries: [],
});

Expand Down Expand Up @@ -1281,6 +1300,8 @@ export function getStrictDiagnosticRuleSet(): DiagnosticRuleSet {
reportUnannotatedClassAttribute: 'none',
reportIncompatibleUnannotatedOverride: 'none',
reportInvalidAbstractMethod: 'none',
reportPackageTypeVerificationError: 'none',
reportPackageTypeVerificationWarning: 'none',
allowedUntypedLibraries: [],
};

Expand Down Expand Up @@ -1346,6 +1367,8 @@ export class ConfigOptions {
//Path to baseline file.
baselineFile?: Uri | undefined;

verifyTypes?: VerifyTypes | undefined;

// A list of file specs to include in the analysis. Can contain
// directories, in which case all "*.py" files within those directories
// are included.
Expand Down
2 changes: 2 additions & 0 deletions packages/pyright-internal/src/common/diagnosticRules.ts
Original file line number Diff line number Diff line change
Expand Up @@ -120,4 +120,6 @@ export enum DiagnosticRule {
reportUnannotatedClassAttribute = 'reportUnannotatedClassAttribute',
reportIncompatibleUnannotatedOverride = 'reportIncompatibleUnannotatedOverride',
reportInvalidAbstractMethod = 'reportInvalidAbstractMethod',
reportPackageTypeVerificationError = 'reportPackageTypeVerificationError',
reportPackageTypeVerificationWarning = 'reportPackageTypeVerificationWarning',
}
29 changes: 16 additions & 13 deletions packages/pyright-internal/src/pyright.ts
Original file line number Diff line number Diff line change
Expand Up @@ -433,6 +433,17 @@ async function processArgs(): Promise<ExitStatus> {

const serviceProvider = createServiceProvider(fileSystem, output, tempFile);

const watch = args.watch !== undefined;
options.languageServerSettings.watchForSourceChanges = watch;
options.languageServerSettings.watchForConfigChanges = watch;

const service = new AnalyzerService('<default>', serviceProvider, {
console: output,
hostFactory: () => new FullAccessHost(serviceProvider),
// Refresh service 2 seconds after the last library file change is detected.
libraryReanalysisTimeProvider: () => 2 * 1000,
});

// The package type verification uses a different path.
if (args['verifytypes'] !== undefined) {
return verifyPackageTypes(
Expand All @@ -441,24 +452,14 @@ async function processArgs(): Promise<ExitStatus> {
options,
!!args.outputjson,
minSeverityLevel,
args['ignoreexternal']
args['ignoreexternal'],
service
);
} else if (args['ignoreexternal'] !== undefined) {
console.error(`'--ignoreexternal' is valid only when used with '--verifytypes'`);
return ExitStatus.ParameterError;
}

const watch = args.watch !== undefined;
options.languageServerSettings.watchForSourceChanges = watch;
options.languageServerSettings.watchForConfigChanges = watch;

const service = new AnalyzerService('<default>', serviceProvider, {
console: output,
hostFactory: () => new FullAccessHost(serviceProvider),
// Refresh service 2 seconds after the last library file change is detected.
libraryReanalysisTimeProvider: () => 2 * 1000,
});

if ('threads' in args) {
let threadCount = args['threads'];

Expand Down Expand Up @@ -909,7 +910,8 @@ function verifyPackageTypes(
options: PyrightCommandLineOptions,
outputJson: boolean,
minSeverityLevel: SeverityLevel,
ignoreUnknownTypesFromImports: boolean
ignoreUnknownTypesFromImports: boolean,
service: AnalyzerService
): ExitStatus {
try {
const host = new FullAccessHost(serviceProvider);
Expand All @@ -918,6 +920,7 @@ function verifyPackageTypes(
host,
options,
packageName,
service.backgroundAnalysisProgram.program,
ignoreUnknownTypesFromImports
);
const report = verifier.verify();
Expand Down
Loading