diff --git a/docs/benefits-over-pyright/new-diagnostic-rules.md b/docs/benefits-over-pyright/new-diagnostic-rules.md index 1f6e2a5a9e..653ff056f9 100644 --- a/docs/benefits-over-pyright/new-diagnostic-rules.md +++ b/docs/benefits-over-pyright/new-diagnostic-rules.md @@ -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. diff --git a/docs/configuration/config-files.md b/docs/configuration/config-files.md index f291a531ea..10b510d66c 100644 --- a/docs/configuration/config-files.md +++ b/docs/configuration/config-files.md @@ -299,6 +299,10 @@ The following settings allow more fine grained control over the **typeCheckingMo - **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) +- **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) + +- **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. diff --git a/packages/pyright-internal/src/analyzer/packageTypeVerifier.ts b/packages/pyright-internal/src/analyzer/packageTypeVerifier.ts index 6f31db61e6..34e60ab100 100644 --- a/packages/pyright-internal/src/analyzer/packageTypeVerifier.ts +++ b/packages/pyright-internal/src/analyzer/packageTypeVerifier.ts @@ -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'; @@ -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) { @@ -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 { diff --git a/packages/pyright-internal/src/analyzer/program.ts b/packages/pyright-internal/src/analyzer/program.ts index f60ffa8e70..91ca584ae1 100644 --- a/packages/pyright-internal/src/analyzer/program.ts +++ b/packages/pyright-internal/src/analyzer/program.ts @@ -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; @@ -139,6 +140,7 @@ export class Program { private _editModeTracker = new EditModeTracker(); private _sourceFileFactory: ISourceFileFactory; + private _packageTypeVerifier?: PackageTypeVerifier; baselineHandler: BaselineHandler; constructor( diff --git a/packages/pyright-internal/src/analyzer/service.ts b/packages/pyright-internal/src/analyzer/service.ts index 89f72c0a61..c5d2f18609 100644 --- a/packages/pyright-internal/src/analyzer/service.ts +++ b/packages/pyright-internal/src/analyzer/service.ts @@ -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( diff --git a/packages/pyright-internal/src/common/commandLineOptions.ts b/packages/pyright-internal/src/common/commandLineOptions.ts index f7b406af3e..62eee119fa 100644 --- a/packages/pyright-internal/src/common/commandLineOptions.ts +++ b/packages/pyright-internal/src/common/commandLineOptions.ts @@ -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. diff --git a/packages/pyright-internal/src/common/configOptions.ts b/packages/pyright-internal/src/common/configOptions.ts index ea816dc163..e45636a863 100644 --- a/packages/pyright-internal/src/common/configOptions.ts +++ b/packages/pyright-internal/src/common/configOptions.ts @@ -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', @@ -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 { @@ -557,6 +564,8 @@ export function getDiagLevelDiagnosticRules() { DiagnosticRule.reportUnannotatedClassAttribute, DiagnosticRule.reportIncompatibleUnannotatedOverride, DiagnosticRule.reportInvalidAbstractMethod, + DiagnosticRule.reportPackageTypeVerificationError, + DiagnosticRule.reportPackageTypeVerificationWarning, ]; } @@ -694,6 +703,8 @@ export function getOffDiagnosticRuleSet(): DiagnosticRuleSet { reportUnannotatedClassAttribute: 'none', reportIncompatibleUnannotatedOverride: 'none', reportInvalidAbstractMethod: 'none', + reportPackageTypeVerificationError: 'none', + reportPackageTypeVerificationWarning: 'none', allowedUntypedLibraries: [], }; @@ -813,6 +824,8 @@ export function getBasicDiagnosticRuleSet(): DiagnosticRuleSet { reportUnannotatedClassAttribute: 'none', reportIncompatibleUnannotatedOverride: 'none', reportInvalidAbstractMethod: 'none', + reportPackageTypeVerificationError: 'none', + reportPackageTypeVerificationWarning: 'none', allowedUntypedLibraries: [], }; @@ -932,6 +945,8 @@ export function getStandardDiagnosticRuleSet(): DiagnosticRuleSet { reportUnannotatedClassAttribute: 'none', reportIncompatibleUnannotatedOverride: 'none', reportInvalidAbstractMethod: 'none', + reportPackageTypeVerificationError: 'none', + reportPackageTypeVerificationWarning: 'none', allowedUntypedLibraries: [], }; @@ -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: [], }); @@ -1165,6 +1182,8 @@ export const getAllDiagnosticRuleSet = (): DiagnosticRuleSet => ({ reportUnannotatedClassAttribute: 'error', reportIncompatibleUnannotatedOverride: 'error', reportInvalidAbstractMethod: 'error', + reportPackageTypeVerificationError: 'error', + reportPackageTypeVerificationWarning: 'error', allowedUntypedLibraries: [], }); @@ -1281,6 +1300,8 @@ export function getStrictDiagnosticRuleSet(): DiagnosticRuleSet { reportUnannotatedClassAttribute: 'none', reportIncompatibleUnannotatedOverride: 'none', reportInvalidAbstractMethod: 'none', + reportPackageTypeVerificationError: 'none', + reportPackageTypeVerificationWarning: 'none', allowedUntypedLibraries: [], }; @@ -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. diff --git a/packages/pyright-internal/src/common/diagnosticRules.ts b/packages/pyright-internal/src/common/diagnosticRules.ts index 634797a5c0..eb4aa92456 100644 --- a/packages/pyright-internal/src/common/diagnosticRules.ts +++ b/packages/pyright-internal/src/common/diagnosticRules.ts @@ -120,4 +120,6 @@ export enum DiagnosticRule { reportUnannotatedClassAttribute = 'reportUnannotatedClassAttribute', reportIncompatibleUnannotatedOverride = 'reportIncompatibleUnannotatedOverride', reportInvalidAbstractMethod = 'reportInvalidAbstractMethod', + reportPackageTypeVerificationError = 'reportPackageTypeVerificationError', + reportPackageTypeVerificationWarning = 'reportPackageTypeVerificationWarning', } diff --git a/packages/pyright-internal/src/pyright.ts b/packages/pyright-internal/src/pyright.ts index 6b8e207535..2bf36178ae 100644 --- a/packages/pyright-internal/src/pyright.ts +++ b/packages/pyright-internal/src/pyright.ts @@ -433,6 +433,17 @@ async function processArgs(): Promise { const serviceProvider = createServiceProvider(fileSystem, output, tempFile); + const watch = args.watch !== undefined; + options.languageServerSettings.watchForSourceChanges = watch; + options.languageServerSettings.watchForConfigChanges = watch; + + const service = new AnalyzerService('', 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( @@ -441,24 +452,14 @@ async function processArgs(): Promise { 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('', 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']; @@ -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); @@ -918,6 +920,7 @@ function verifyPackageTypes( host, options, packageName, + service.backgroundAnalysisProgram.program, ignoreUnknownTypesFromImports ); const report = verifier.verify();