From c9622787039d689dd65bbef9562d54046c407dd8 Mon Sep 17 00:00:00 2001 From: detachhead Date: Tue, 13 Feb 2024 14:13:09 +1000 Subject: [PATCH 1/3] remove support for relative imports that are not prefixed with a `.` --- .../src/analyzer/importResolver.ts | 72 +------------------ .../completions.parentFolder.fourslash.ts | 2 +- .../src/tests/importResolver.test.ts | 18 +---- 3 files changed, 6 insertions(+), 86 deletions(-) diff --git a/packages/pyright-internal/src/analyzer/importResolver.ts b/packages/pyright-internal/src/analyzer/importResolver.ts index a33f2912e0..4c174c2f04 100644 --- a/packages/pyright-internal/src/analyzer/importResolver.ts +++ b/packages/pyright-internal/src/analyzer/importResolver.ts @@ -25,7 +25,7 @@ import { getFileSystemEntriesFromDirEntries, isDirectory, isFile, tryRealpath, t import { isIdentifierChar, isIdentifierStartChar } from '../parser/characters'; import { ImplicitImport, ImportResult, ImportType } from './importResult'; import { getDirectoryLeadingDotsPointsTo } from './importStatementUtils'; -import { ImportPath, ParentDirectoryCache } from './parentDirectoryCache'; +import { ParentDirectoryCache } from './parentDirectoryCache'; import { PyTypedInfo, getPyTypedInfo } from './pyTypedUtils'; import * as PythonPathUtils from './pythonPathUtils'; import * as SymbolNameUtils from './symbolNameUtils'; @@ -535,75 +535,7 @@ export class ImportResolver { ): ImportResult { const importName = this.formatImportName(moduleDescriptor); const importFailureInfo: string[] = []; - const importResult = this._resolveImportStrict( - importName, - sourceFileUri, - execEnv, - moduleDescriptor, - importFailureInfo - ); - - if (importResult.isImportFound || moduleDescriptor.leadingDots > 0) { - return importResult; - } - - // If the import is absolute and no other method works, try resolving the - // absolute in the importing file's directory, then the parent directory, - // and so on, until the import root is reached. - const origin = sourceFileUri.getDirectory(); - - const result = this.cachedParentImportResults.getImportResult(origin, importName, importResult); - if (result) { - // Already ran the parent directory resolution for this import name on this location. - return this.filterImplicitImports(result, moduleDescriptor.importedSymbols); - } - - // Check whether the given file is in the parent directory import resolution cache. - const root = this.getParentImportResolutionRoot(sourceFileUri, execEnv.root); - if (!this.cachedParentImportResults.checkValidPath(this.fileSystem, sourceFileUri, root)) { - return importResult; - } - - const importPath: ImportPath = { importPath: undefined }; - - // Going up the given folder one by one until we can resolve the import. - let current: Uri | undefined = origin; - while (this._shouldWalkUp(current, root, execEnv) && current) { - const result = this.resolveAbsoluteImport( - sourceFileUri, - current, - execEnv, - moduleDescriptor, - importName, - [], - /* allowPartial */ undefined, - /* allowNativeLib */ undefined, - /* useStubPackage */ false, - /* allowPyi */ true - ); - - this.cachedParentImportResults.checked(current!, importName, importPath); - - if (result.isImportFound) { - // This will make cache to point to actual path that contains the module we found - importPath.importPath = current; - - this.cachedParentImportResults.add({ - importResult: result, - path: current, - importName, - }); - - return this.filterImplicitImports(result, moduleDescriptor.importedSymbols); - } - - current = this._tryWalkUp(current); - } - - if (current) { - this.cachedParentImportResults.checked(current, importName, importPath); - } - return importResult; + return this._resolveImportStrict(importName, sourceFileUri, execEnv, moduleDescriptor, importFailureInfo); } protected fileExistsCached(uri: Uri): boolean { diff --git a/packages/pyright-internal/src/tests/fourslash/completions.parentFolder.fourslash.ts b/packages/pyright-internal/src/tests/fourslash/completions.parentFolder.fourslash.ts index ef9cc90ab6..ef048c7454 100644 --- a/packages/pyright-internal/src/tests/fourslash/completions.parentFolder.fourslash.ts +++ b/packages/pyright-internal/src/tests/fourslash/completions.parentFolder.fourslash.ts @@ -10,7 +10,7 @@ //// #empty // @ts-ignore -await helper.verifyCompletion('included', 'markdown', { +await helper.verifyCompletion('excluded', 'markdown', { marker: { completions: [{ label: 'data_processing', kind: Consts.CompletionItemKind.Module }], }, diff --git a/packages/pyright-internal/src/tests/importResolver.test.ts b/packages/pyright-internal/src/tests/importResolver.test.ts index 241bc5aa5f..e2cd17ef10 100644 --- a/packages/pyright-internal/src/tests/importResolver.test.ts +++ b/packages/pyright-internal/src/tests/importResolver.test.ts @@ -525,11 +525,7 @@ describe('Import tests that can run with or without a true venv', () => { ]; const importResult = getImportResult(files, ['file1']); - assert(importResult.isImportFound); - assert.strictEqual( - 1, - importResult.resolvedUris.filter((f) => f.getFilePath() === combinePaths('/test', 'file1.py')).length - ); + assert(!importResult.isImportFound); }); test('import side by side file sub under src folder', () => { @@ -545,11 +541,7 @@ describe('Import tests that can run with or without a true venv', () => { ]; const importResult = getImportResult(files, ['file1']); - assert(importResult.isImportFound); - assert.strictEqual( - 1, - importResult.resolvedUris.filter((f) => f.getFilePath() === combinePaths('/src/nested', 'file1.py')).length - ); + assert(!importResult.isImportFound); }); test('import file sub under containing folder', () => { @@ -565,11 +557,7 @@ describe('Import tests that can run with or without a true venv', () => { ]; const importResult = getImportResult(files, ['file1']); - assert(importResult.isImportFound); - assert.strictEqual( - 1, - importResult.resolvedUris.filter((f) => f.getFilePath() === combinePaths('/src/nested', 'file1.py')).length - ); + assert(!importResult.isImportFound); }); test("don't walk up the root", () => { From d9149c804935698f892fec789ae4d88909a4b595 Mon Sep 17 00:00:00 2001 From: detachhead Date: Tue, 13 Feb 2024 14:42:05 +1000 Subject: [PATCH 2/3] switch to my fork of mypy_primer --- .github/workflows/mypy_primer_pr.yaml | 2 +- .github/workflows/mypy_primer_push.yaml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/mypy_primer_pr.yaml b/.github/workflows/mypy_primer_pr.yaml index 12518680ef..3565cca909 100644 --- a/.github/workflows/mypy_primer_pr.yaml +++ b/.github/workflows/mypy_primer_pr.yaml @@ -48,7 +48,7 @@ jobs: - name: Install dependencies run: | python -m pip install -U pip - pip install git+https://github.com/hauntsaninja/mypy_primer.git + pip install git+https://github.com/detachhead/mypy_primer.git - name: Run mypy_primer shell: bash run: | diff --git a/.github/workflows/mypy_primer_push.yaml b/.github/workflows/mypy_primer_push.yaml index 17351fec4f..82aced2947 100644 --- a/.github/workflows/mypy_primer_push.yaml +++ b/.github/workflows/mypy_primer_push.yaml @@ -39,7 +39,7 @@ jobs: - name: Install dependencies run: | python -m pip install -U pip - pip install git+https://github.com/hauntsaninja/mypy_primer.git + pip install git+https://github.com/detachhead/mypy_primer.git - name: Run mypy_primer shell: bash run: | From 930487f6009c6f80c4e477db7a5efbb7f088f64e Mon Sep 17 00:00:00 2001 From: detachhead Date: Tue, 13 Feb 2024 16:00:42 +1000 Subject: [PATCH 3/3] re-enable fake relative imports on namespace packages and stubs --- .../src/analyzer/importResolver.ts | 75 ++++++++++++++++++- 1 file changed, 73 insertions(+), 2 deletions(-) diff --git a/packages/pyright-internal/src/analyzer/importResolver.ts b/packages/pyright-internal/src/analyzer/importResolver.ts index 4c174c2f04..5abc9d57c6 100644 --- a/packages/pyright-internal/src/analyzer/importResolver.ts +++ b/packages/pyright-internal/src/analyzer/importResolver.ts @@ -25,7 +25,7 @@ import { getFileSystemEntriesFromDirEntries, isDirectory, isFile, tryRealpath, t import { isIdentifierChar, isIdentifierStartChar } from '../parser/characters'; import { ImplicitImport, ImportResult, ImportType } from './importResult'; import { getDirectoryLeadingDotsPointsTo } from './importStatementUtils'; -import { ParentDirectoryCache } from './parentDirectoryCache'; +import { ImportPath, ParentDirectoryCache } from './parentDirectoryCache'; import { PyTypedInfo, getPyTypedInfo } from './pyTypedUtils'; import * as PythonPathUtils from './pythonPathUtils'; import * as SymbolNameUtils from './symbolNameUtils'; @@ -535,7 +535,78 @@ export class ImportResolver { ): ImportResult { const importName = this.formatImportName(moduleDescriptor); const importFailureInfo: string[] = []; - return this._resolveImportStrict(importName, sourceFileUri, execEnv, moduleDescriptor, importFailureInfo); + const importResult = this._resolveImportStrict( + importName, + sourceFileUri, + execEnv, + moduleDescriptor, + importFailureInfo + ); + + if (importResult.isImportFound || moduleDescriptor.leadingDots > 0) { + return importResult; + } + + // If the import is absolute and no other method works, try resolving the + // absolute in the importing file's directory, then the parent directory, + // and so on, until the import root is reached. + const origin = sourceFileUri.getDirectory(); + + const result = this.cachedParentImportResults.getImportResult(origin, importName, importResult); + if (result) { + // Already ran the parent directory resolution for this import name on this location. + return this.filterImplicitImports(result, moduleDescriptor.importedSymbols); + } + + // Check whether the given file is in the parent directory import resolution cache. + const root = this.getParentImportResolutionRoot(sourceFileUri, execEnv.root); + if (!this.cachedParentImportResults.checkValidPath(this.fileSystem, sourceFileUri, root)) { + return importResult; + } + + const importPath: ImportPath = { importPath: undefined }; + + // Going up the given folder one by one until we can resolve the import. + let current: Uri | undefined = origin; + while (this._shouldWalkUp(current, root, execEnv) && current) { + const result = this.resolveAbsoluteImport( + sourceFileUri, + current, + execEnv, + moduleDescriptor, + importName, + [], + /* allowPartial */ undefined, + /* allowNativeLib */ undefined, + /* useStubPackage */ false, + /* allowPyi */ true + ); + + this.cachedParentImportResults.checked(current!, importName, importPath); + + // TODO: figure out how namespace packages work. we disable this stupid fake relative import functionality because it's wrong + // (https://github.com/DetachHead/basedpyright/issues/76) but it seems like stub imports and some mysterious namespace package + // functionality rely on this behavior. so for now i'm just disabling it for everything except namespace packages & stub files + if (result.isImportFound && (result.isNamespacePackage || result.isStubFile)) { + // This will make cache to point to actual path that contains the module we found + importPath.importPath = current; + + this.cachedParentImportResults.add({ + importResult: result, + path: current, + importName, + }); + + return this.filterImplicitImports(result, moduleDescriptor.importedSymbols); + } + + current = this._tryWalkUp(current); + } + + if (current) { + this.cachedParentImportResults.checked(current, importName, importPath); + } + return importResult; } protected fileExistsCached(uri: Uri): boolean {