Skip to content
This repository was archived by the owner on Apr 14, 2022. It is now read-only.

PLS doesn't flag error in a relative import if it is found in another root directory #509

Closed
erictraut opened this issue Dec 26, 2018 · 2 comments
Assignees

Comments

@erictraut
Copy link

erictraut commented Dec 26, 2018

The analyzer incorrectly accepts relative imports in cases where the path is relative to a root but not to the importing file.

Consider the following case:
Directory structure:
src
|->module
|--->init.py
test.py

PYTHONPATH=src

test.py contains the following code:

from .module import Foo

This import should be flagged as invalid because the path ".module" should be relative to the test.py file. However, the code in PathResolverSnapshot.GetImportsFromRelativePath considers all of the root search directories when it performs its search. It therefore finds an import match when it shouldn't.

I think the problem is in PathResolverSnapshot.RootContains. It doesn't do anything to validate that the specified lastEdge starts in the same location as the root. If the relative path happens to match items within that root, the method returns true. If the relative path is zero length, all roots return true.

A proposed fix is to add the following piece of code to PathResolverSnapshot.RootContains before the while loop:

if (root != sourceEdge.Next.Start) {
    rootEdge = default;
    return false;
}
@AlexanderSher
Copy link
Contributor

AlexanderSher commented Dec 31, 2018

The problem isn't related to the multiple roots. With "python.autoComplete.extraPaths": ["path_to_folder/src"] and the following file structure:

├─ package
│  └─ test.py
└─ src
   └─ package
      └─ module
         └─ __init__.py

code from .module import * in test.py is valid.

But, from .module is invalid when test.py is in root, no matter if we have module or not.

@AlexanderSher
Copy link
Contributor

AlexanderSher commented Dec 31, 2018

We probably should have an exception for the working directory, cause it can be a top directory of the module.

AlexanderSher added a commit to AlexanderSher/python-language-server that referenced this issue Jan 2, 2019
…t is found in another root directory

- Fix microsoft#510: PLS doesn't resolve relative paths correctly
AlexanderSher added a commit that referenced this issue Jan 2, 2019
- Fix #509: PLS doesn't flag error in a relative import if it is found in another root directory (#519)
- Fix #510: PLS doesn't resolve relative paths correctly
AlexanderSher added a commit to AlexanderSher/python-language-server that referenced this issue Feb 4, 2019
- Fix microsoft#509: PLS doesn't flag error in a relative import if it is found in another root directory (microsoft#519)
- Fix microsoft#510: PLS doesn't resolve relative paths correctly
AlexanderSher added a commit that referenced this issue Feb 4, 2019
* Fix #470 (#478)

* Fix various common exceptions, log on a failed directory load (#498)

* rethrow exception after telemetry instead of disposing

* log an error instead of crashing when hitting some exceptions loading files, fix nullref when shutting down without an idle tracker

* fix short circuiting of question mark checks to prevent null from being returned

* print name of exception instead of relying on ToString

* don't use MaybeEnumerate

* upgrade message to Show

* Mitigate some of #495 via MRO member selection and analysis set optimization (#517)

This isn't a complete fix, but does seem to improve #495 in some cases. Adds back the early MRO return removed in #277, so now large class hierarchies won't over-propagate types (where some of the trouble with fastai happens do to the Transform class). I also optimized AnalysisHashSet a little to do length checks when possible.

There are still times when things get caught up comparing unions for a while, but it seems to be nondeterministic.

* Two path resolution bug fixes

- Fix #509: PLS doesn't flag error in a relative import if it is found in another root directory (#519)
- Fix #510: PLS doesn't resolve relative paths correctly

* Catch exceptions when importing from search paths (#525)

* catch exceptions when importing from search paths

* retry instead of not found
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants