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.

Sign up for GitHub

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

Null checks & Issue with resolution of "Unknown.hx" #936

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

Null checks & Issue with resolution of "Unknown.hx" #936

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 8 additions & 5 deletions src/common/com/intellij/plugins/haxe/lang/psi/HaxeResolver.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -430,11 +430,14 @@ private static List<? extends PsiElement> resolveByClassAndSymbol(@Nullable Haxe
if (leftClass.isAbstract()) {
HaxeAbstractClassModel model = (HaxeAbstractClassModel)leftClass.getModel();
if (model.isForwarded(reference.getReferenceName())) {
final HaxeClass underlyingClass = model.getUnderlyingClass(reference.getSpecialization().toGenericResolver(leftClass));
if (underlyingClass != null) {
member = underlyingClass.getModel().getMember(reference.getReferenceName());
if (member != null) {
return asList(member.getNamePsi());
HaxeGenericSpecialization specialization = reference.getSpecialization();
if(specialization !=null){
final HaxeClass underlyingClass = model.getUnderlyingClass(specialization.toGenericResolver(leftClass));
if (underlyingClass != null) {
member = underlyingClass.getModel().getMember(reference.getReferenceName());
if (member != null) {
return asList(member.getNamePsi());
}
}
}
}
Expand Down
5 changes: 2 additions & 3 deletions src/common/com/intellij/plugins/haxe/lang/psi/impl/HaxeReferenceImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -230,11 +230,10 @@ private JavaResolveResult[] multiResolve(boolean incompleteCode, boolean resolve
@Nullable
public PsiElement resolveToComponentName() {
final ResolveResult[] resolveResults = multiResolve(true, false);
final PsiElement result = resolveResults.length == 0 ||
resolveResults.length > 1 ||
final PsiElement result = resolveResults.length != 1 ||
!resolveResults[0].isValidResult() ? null : resolveResults[0].getElement();

if (result != null && result instanceof HaxeNamedComponent) {
if (result instanceof HaxeNamedComponent) {
return ((HaxeNamedComponent)result).getComponentName();
}

Expand Down
20 changes: 17 additions & 3 deletions src/common/com/intellij/plugins/haxe/model/HaxePackageModel.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,9 +125,23 @@ public HaxeFileModel getFileModel(String fileName) {
protected HaxeFile getFile(String fileName) {
PsiDirectory directory = root.access(path);
if (directory != null && directory.isValid()) {
PsiFile file = directory.findFile(fileName + ".hx");
if (file != null && file.isValid() && file instanceof HaxeFile) {
return (HaxeFile)file;
PsiFile file;
try {
String fName = fileName + ".hx";
//TODO @ Eric. What could be the source of a request for "Unknown.hx" ?
Copy link
Member

Choose a reason for hiding this comment

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

There is a magic type of "unknown" in the language that is assigned to variables before type inference is able to set the type. (It's the same in the compiler -- just trace(some_uninitialized_var); to see it pop out.) If the resolver gets a variable in this condition, it's going to try and find the class by upper-casing the name and looking for an implementation file. Were we throwing an exception out of this function? What was the bug that made you look here?

(BTW, ProcessCanceledExceptions are expected and should cancel the current task. That's the way that IntelliJ deals with blocked/blocking tasks.)

Copy link
Author

Choose a reason for hiding this comment

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

Yeah whenever it tried to load "Unknown.hx" it was throwing. The Intellij stack trace got me there eventually.

Copy link
Author

Choose a reason for hiding this comment

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

Oh I just noticed your BTW. That may be true but when it threw here it stopped parsing code. This is evident if you can repro from my instructions in the commit notes.

Copy link
Member

@EricBishton EricBishton Jul 10, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

I've been unable to repro the exception being thrown and parsing stopping. If I set a breakpoint in getFile(), I can definitely see the code trying to find the file -- every time an unknown class reference is created. Issue #938 is for that.

if(fName.equals("Unknown.hx")){
Copy link
Member

Choose a reason for hiding this comment

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

BTW, there is nothing in the language that says you can't have a class called Unknown and an Unknown.hx file to go with it. So, just returning null when we see that name is going to create other bugs.

Perhaps we should rename our internal unknown class to __unknown__? The double leading underscore is soft-reserved for compiler internals anyway. Unfortunately, I'm sure that it will pop out to the UI somewhere...

Copy link
Author

Choose a reason for hiding this comment

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

I agree...I didnt expect my little hack to be permanent.. Shall I try to find that magic type and rename it as you recommended?

return null;
}
//file = directory.findFile(fileName + ".hx");
file = directory.findFile(fName);
if (file != null && file.isValid() && file instanceof HaxeFile) {
return (HaxeFile)file;
}
} catch(Exception e){
//System.out.println("-----------------------------------");
//System.out.println(fileName + ".hx");
//System.out.println("-----------------------------------");
System.out.println(Arrays.toString(e.getStackTrace()));
Copy link
Member

Choose a reason for hiding this comment

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

Always use the HaxeDebugLogger instead of any of the System io dumps (out, err); the System calls can create write dead-locks when multiple JetBrains products are used simultaneously or when called on background threads (for example, see #724). They are also not captured in user logs where they could be useful for debugging.

HaxeDebugLogger.getLogger().warn("Message"); is the easy way to add a temporary message. There are many examples of more interesting usage throughout the code base.

Copy link
Author

Choose a reason for hiding this comment

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

Cool..thanks for that tip.

}
}

Expand Down