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

Implemented allowedUntypedLibraries config #970

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Wizzerinus
Copy link

@Wizzerinus Wizzerinus commented Dec 27, 2024

Basically, this PR adds an allowedUntypedLibraries config field which is a list of strings (empty by default). If pyright cannot derive the type of a function, it normally gives an error (reportUnknownMemberType or reportUnknownVariableType), and if the library the method is implemented in is in this list, it doesn't, for example:

# both foo and bar are untyped, `allowedUntypedLibraries = ["first"]`
from first import foo
from second import bar
foo("123")   # ok
bar("456")   # error

Notes:

  • Not documented right now
  • maybe it's better to name this allowedUntypedModules instead of libraries
  • maybe it would be good to allow configuring a list of modules deeper than 1 level (i.e. "library is strongly typed but library.new_submodule isn't"), although I think this is not necessary due to the uses of this feature (the point is that the rule normally targets the code written for the project, and this PR targets the code written for a library used in the project, which are usually not intersecting)

resolves #701

This comment has been minimized.

Copy link
Owner

@DetachHead DetachHead left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there are probably other things we will need to consider as well that haven't yet crossed my mind

packages/pyright-internal/src/baseline.ts Outdated Show resolved Hide resolved
packages/pyright-internal/src/common/configOptions.ts Outdated Show resolved Hide resolved

This comment has been minimized.

@Wizzerinus Wizzerinus force-pushed the feature/config/allowed-untyped-libraries branch from 5b2c8f3 to 665bc94 Compare January 9, 2025 15:50
@Wizzerinus Wizzerinus force-pushed the feature/config/allowed-untyped-libraries branch from 665bc94 to e302c06 Compare January 9, 2025 15:53
Comment on lines +406 to +411
// If the module is allowed as an untyped library, we don't need the stub
if (
!importResult.isStubFile &&
importResult.importType === ImportType.ThirdParty &&
!importResult.pyTypedInfo
!importResult.pyTypedInfo &&
!this._ignoreUntypedModule(importResult.importName)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: maybe move this comment to be directly above that new condition so it's more clear which part of the if statement it's referring to:

        if (
            !importResult.isStubFile &&
            importResult.importType === ImportType.ThirdParty &&
            !importResult.pyTypedInfo &&
            // If the module is allowed as an untyped library, we don't need the stub
            !this._ignoreUntypedModule(importResult.importName)
        ) {}

@@ -15114,6 +15114,10 @@ export function createTypeEvaluator(
return { type, isIncomplete, typeErrors };
}

function _ignoreUntypedModule(ruleset: DiagnosticRuleSet, module: string) {
return ruleset.allowedUntypedLibraries.some(x => (module + ".").startsWith(x + "."));
Copy link
Owner

@DetachHead DetachHead Jan 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since this logic is repeated, perhaps it can be abstracted into a utility function located in ./packages/pyright-internal/src/analyzer/importStatementUtils.ts or something? (maybe thats not the best place for it so feel free to put it somewhere else if you can find a better spot)

Copy link
Contributor

github-actions bot commented Jan 9, 2025

Diff from mypy_primer, showing the effect of this PR on open source code:

AutoSplit (https://github.com/Toufool/AutoSplit): 198.02x slower (0.1s -> 18.3s in a single noisy sample)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Apply different typing rules to particular (external) packages
2 participants