Skip to content

Commit

Permalink
Imports found inside a typing.TYPE_CHECKING will be considered undefi…
Browse files Browse the repository at this point in the history 
…ned if the scope that uses it requires it to be available when not type-checking.
  • Loading branch information
@fabioz
fabioz committed Oct 1, 2023
1 parent ab66dee commit b04031d
Show file tree
Hide file tree
Showing 6 changed files with 211 additions and 43 deletions.
45 changes: 32 additions & 13 deletions ...ev.analysis/src/com/python/pydev/analysis/scopeanalysis/AbstractScopeAnalyzerVisitor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -415,7 +415,11 @@ public Object visitFunctionDef(FunctionDef node) throws Exception {
if (args.annotation != null) {
for (exprType expr : args.annotation) {
if (expr != null) {
startScope(Scope.SCOPE_TYPE_ANNOTATION, expr);
int scopeType = Scope.SCOPE_TYPE_ANNOTATION;
if (futureAnnotationsImported) {
scopeType = Scope.SCOPE_TYPE_ANNOTATION_STR;
}
startScope(scopeType, expr);
expr.accept(visitor);
endScope(expr);
}
Expand All @@ -424,7 +428,13 @@ public Object visitFunctionDef(FunctionDef node) throws Exception {

//visit the return
if (node.returns != null) {
int scopeType = Scope.SCOPE_TYPE_ANNOTATION;
if (futureAnnotationsImported) {
scopeType = Scope.SCOPE_TYPE_ANNOTATION_STR;
}
startScope(scopeType, node.returns);
node.returns.accept(visitor);
endScope(node.returns);
}

//then the decorators (no, still not in method scope)
Expand Down Expand Up @@ -886,9 +896,9 @@ protected static void visitCallAttr(Call c, VisitorBase base) throws Exception {

@Override
public Object visitFor(For node) throws Exception {
scope.addIfSubScope();
scope.addStatementSubScope();
Object ret = super.visitFor(node);
scope.removeIfSubScope();
scope.removeStatementSubScope();
return ret;
}

Expand All @@ -905,7 +915,11 @@ public Object visitAssign(Assign node) throws Exception {
}

if (node.type != null) {
startScope(Scope.SCOPE_TYPE_ANNOTATION, node.type);
int scopeType = Scope.SCOPE_TYPE_ANNOTATION;
if (futureAnnotationsImported) {
scopeType = Scope.SCOPE_TYPE_ANNOTATION_STR;
}
startScope(scopeType, node.type);
node.type.accept(this);
endScope(node.type);
}
Expand All @@ -927,7 +941,7 @@ public Object visitAssign(Assign node) throws Exception {
*/
@Override
public Object visitIf(If node) throws Exception {
scope.addIfSubScope();
scope.addIfSubScope(node);
Object r = super.visitIf(node);
scope.removeIfSubScope();
return r;
Expand All @@ -938,9 +952,9 @@ public Object visitIf(If node) throws Exception {
*/
@Override
public Object visitWhile(While node) throws Exception {
scope.addIfSubScope();
scope.addStatementSubScope();
Object r = super.visitWhile(node);
scope.removeIfSubScope();
scope.removeStatementSubScope();
return r;
}

Expand All @@ -954,9 +968,9 @@ public Object visitTryExcept(TryExcept node) throws Exception {

@Override
public Object visitTryFinally(TryFinally node) throws Exception {
scope.addIfSubScope();
scope.addStatementSubScope();
Object r = super.visitTryFinally(node);
scope.removeIfSubScope();
scope.removeStatementSubScope();
return r;
}

Expand Down Expand Up @@ -1116,14 +1130,17 @@ protected void endScope(SimpleNode node) {
for (Found found : foundItems) {
//the scope where it is defined must be an outer scope so that we can say it was defined later...
final GenAndTok foundItemFirst = found.getSingle();

//if something was not defined in a method, if we are in the class definition, it won't be found.

if ((probablyNotDefinedFirst.scopeFound.getScopeType() & Scope.ACCEPTED_METHOD_AND_LAMBDA) != 0
&& m.getScopeType() != Scope.SCOPE_TYPE_CLASS) {
if (foundItemFirst.scopeId < probablyNotDefinedFirst.scopeId) {
found.setUsed(true);
setUsed = true;
if (scope.typeCheckingDefinitionAndUsageOk(foundItemFirst,
probablyNotDefinedFirst.inTypeChecking,
probablyNotDefinedFirst.scopeFound.getScopeType())) {
found.setUsed(true);
setUsed = true;
}
}
}
}
Expand Down Expand Up @@ -1186,6 +1203,7 @@ protected boolean markRead(IToken token, String rep, boolean addToNotDefined, bo

int acceptedScopes = 0;
ScopeItems currScopeItems = scope.getCurrScopeItems();
boolean inTypeChecking = currScopeItems.isInTypeChecking();

if ((currScopeItems.getScopeType() & Scope.ACCEPTED_METHOD_AND_LAMBDA) != 0) {
acceptedScopes = Scope.ACCEPTED_METHOD_SCOPES;
Expand All @@ -1210,7 +1228,8 @@ protected boolean markRead(IToken token, String rep, boolean addToNotDefined, bo
//search for it
while (found == false && it.hasNext()) {
String nextTokToSearch = it.next();
foundAs = scope.findFirst(nextTokToSearch, true, acceptedScopes);
foundAs = scope.findFirst(nextTokToSearch, true, acceptedScopes, inTypeChecking,
currScopeItems.getScopeType());
found = foundAs != null;
if (found) {
foundAsStr = nextTokToSearch;
Expand Down
2 changes: 1 addition & 1 deletion .../com.python.pydev.analysis/src/com/python/pydev/analysis/visitors/DuplicationChecker.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ private boolean canReplaceScope(SimpleNode currNode, SimpleNode node) {
*/
private void checkDuplication(String name, SimpleNode node) {
if (stack.size() > 0) {
if (!scope.getPrevScopeItems().getIsInSubSubScope()) {
if (!scope.getPrevScopeItems().getIsInStatementSubScope()) {
SimpleNode currNode = stack.peek().get(name);
if (currNode != null) {
if (hasTypingOverloadDecorator(currNode) || hasTypingOverloadDecorator(node)) {
Expand Down
8 changes: 7 additions & 1 deletion plugins/com.python.pydev.analysis/src/com/python/pydev/analysis/visitors/GenAndTok.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ public final class GenAndTok {

/**
* This is the token that is from the current module that created the token (if on some wild import)
*
*
* May be equal to tok
*/
public final IToken generator;
Expand All @@ -44,11 +44,17 @@ public final class GenAndTok {
*/
public final ScopeItems scopeFound;

/**
* Determines whether it was in a typing.TYPE_CHECKING if when it was found.
*/
public final boolean inTypeChecking;

public GenAndTok(IToken generator, IToken tok, int scopeId, ScopeItems scopeFound) {
this.generator = generator;
this.tok = tok;
this.scopeId = scopeId;
this.scopeFound = scopeFound;
this.inTypeChecking = scopeFound.isInTypeChecking();
}

@Override
Expand Down
64 changes: 46 additions & 18 deletions plugins/com.python.pydev.analysis/src/com/python/pydev/analysis/visitors/Scope.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import org.python.pydev.core.IToken;
import org.python.pydev.core.IterTokenEntry;
import org.python.pydev.core.TokensList;
import org.python.pydev.parser.jython.ast.If;
import org.python.pydev.parser.jython.ast.TryExcept;
import org.python.pydev.shared_core.string.FastStringBuffer;
import org.python.pydev.shared_core.structure.FastStack;
Expand Down Expand Up @@ -176,7 +177,8 @@ public Scope(AbstractScopeAnalyzerVisitor visitor, IPythonNature nature, String
* - imports such as import os.path (one token is created for os and one for os.path)
*/
public void addImportTokens(TokensList list, IToken generator, ICompletionCache completionCache) {
ScopeItems.TryExceptInfo withinExceptNode = scope.peek().getTryExceptImportError();
ScopeItems currScope = scope.peek();
ScopeItems.TryExceptInfo withinExceptNode = currScope.getTryExceptImportError();

//only report undefined imports if we're not inside a try..except ImportError.
boolean reportUndefinedImports = withinExceptNode == null;
Expand All @@ -197,11 +199,10 @@ public void addImportTokens(TokensList list, IToken generator, ICompletionCache
requireTokensToBeImports = true;
}

ScopeItems m = scope.peek();
for (IterTokenEntry entry : list) {
IToken o = entry.getToken();
//System.out.println("adding: "+o.getRepresentation());
Found found = addToken(generator, m, o, o.getRepresentation());
Found found = addToken(generator, currScope, o, o.getRepresentation());
if (withinExceptNode != null) {
withinExceptNode.addFoundImportToTryExcept(found); //may mark previous as used...
}
Expand Down Expand Up @@ -253,16 +254,18 @@ public Found addToken(IToken generator, ScopeItems m, IToken o, String rep) {
generator = o;
}

Found found = findFirst(rep, false);
Found found = findFirst(rep, false, m.isInTypeChecking(), m.getScopeType());

boolean isReimport = false;
if (!isInMethodDefinition && found != null) { //it will be removed from the scope
if (found.isImport() && generator.isImport()) {
isReimport = true;
if (!found.getSingle().inTypeChecking) {
isReimport = true;
}
//keep on going, as it still might be used or unused

} else {
if (!found.isUsed() && !m.getIsInSubSubScope()) { // it was not used, and we're not in an if scope...
if (!found.isUsed() && !m.getIsInStatementSubScope()) { // it was not used, and we're not in an if scope...

//this kind of unused message should only happen if we are at the same scope...
if (found.getSingle().scopeFound.getScopeId() == getCurrScopeId()) {
Expand Down Expand Up @@ -290,6 +293,9 @@ public Found addToken(IToken generator, ScopeItems m, IToken o, String rep) {
}

Found newFound = new Found(o, generator, m.getScopeId(), m);
if (m.isInTypeChecking()) {
newFound.setUsed(true); // Don't report imports inside of typing.TYPE_CHECKING as unused.
}
if (isReimport) {
if (m.getTryExceptImportError() == null) {
//we don't want to add reimport messages if we're within a try..except
Expand Down Expand Up @@ -377,39 +383,61 @@ public List<Found> findInScopes(String name, boolean setUsed) {
return ret;
}

public Found findFirst(String name, boolean setUsed) {
return findFirst(name, setUsed, ACCEPTED_ALL_SCOPES);
public Found findFirst(String name, boolean setUsed, boolean inTypeChecking, int currScopeType) {
return findFirst(name, setUsed, ACCEPTED_ALL_SCOPES, inTypeChecking, currScopeType);
}

public boolean typeCheckingDefinitionAndUsageOk(GenAndTok definition, boolean usageTypeChecking,
int currScopeType) {
// This if is not needed because a SCOPE_TYPE_ANNOTATION_STR is used instead of a SCOPE_TYPE_ANNOTATION
// when visitor.futureAnnotationsImported == true.
// if (visitor.futureAnnotationsImported) {
// }
if ((currScopeType & SCOPE_TYPE_ANNOTATION_STR) != 0) {
return true;
}
if (definition.inTypeChecking && !usageTypeChecking) {
return false;
}
return true;
}

public Found findFirst(String name, boolean setUsed, int acceptedScopes) {
public Found findFirst(String name, boolean setUsed, int acceptedScopes, boolean inTypeChecking,
int currScopeType) {
Iterator<ScopeItems> topDown = scope.topDownIterator();
while (topDown.hasNext()) {
ScopeItems m = topDown.next();
if ((m.getScopeType() & acceptedScopes) != 0) {
Found f = m.getLastAppearance(name);
if (f != null) {
if (setUsed) {
f.setUsed(true);
if (typeCheckingDefinitionAndUsageOk(f.getSingle(), inTypeChecking, currScopeType)) {
if (setUsed) {
f.setUsed(true);
}
return f;
}
return f;
}
}
}
return null;
}

public void addIfSubScope() {
scope.peek().addIfSubScope();
}

public boolean getIsInIfSubScope() {
return scope.peek().getIsInIfSubScope();
public void addIfSubScope(If node) {
scope.peek().addIfSubScope(node);
}

public void removeIfSubScope() {
scope.peek().removeIfSubScope();
}

public void addStatementSubScope() {
scope.peek().addStatementSubScope();
}

public void removeStatementSubScope() {
scope.peek().removeStatementSubScope();
}

public void addTryExceptSubScope(TryExcept node) {
scope.peek().addTryExceptSubScope(node);
}
Expand Down
Loading

0 comments on commit b04031d

Please sign in to comment.