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

Detect method return types not just by typetag signature #288

Closed
soywiz opened this issue Jun 3, 2015 · 11 comments
Closed

Detect method return types not just by typetag signature #288

soywiz opened this issue Jun 3, 2015 · 11 comments

Comments

@soywiz
Copy link
Contributor

soywiz commented Jun 3, 2015

Today I was able to successfully compile intelliJ haxe plugin and be able to debug it and play a bit with it. And I want to collaborate (I have already forked the project).

Some stuff is a little mess. And requires some refactoring.
I ended in AbstractHaxeNamedComponent.getPresentation() that is used when IDE displays list of members for example using "this." I have seen that it search for a HaxeTypeTag instance, so that display won't be able to present successfully in other cases.

So I would start using HaxeMethodPsiMixinImpl.getTypeTag initially, and then having a proper method that analyzes the AST to get the return type.

If you could help me with these questions it would be easier for me:
Is already code that analyzes psi nodes to get an haxe type?
var a = this.method(); // where is resolved the a type obtained from the method? So I can updated that code too.

@EBatTiVo
Copy link
Contributor

EBatTiVo commented Jun 3, 2015

Some stuff is a little mess. And requires some refactoring.

I'll only take partial credit for that. ;-)

There is nothing that parses the AST, as such to determine types. There is code in the PSI that tries to resolve to a particular type. (If you don't understand the difference between the two, then you need to read the IntelliJ plugin development documentation., The short story is that the AST equates to the individual lexemes and the PSI contains the language semantics.) The file HaxeResolveUtil is the code that attempts to figure out typing in order to find the instance of a given ID (as opposed to a /reference/ to that ID). There is NO code that attempts to analyze the PSI to determine the type of a Dynamic variable at any given point. It's an interesting problem, but one that has already been solved by the Haxe compiler, so our (TiVo's) effort is going toward integration with the compiler, which will eventually -- hopefully soon -- replace the current resolution code entirely.

I will warn you that the resolution code is a huge mess that nobody understands clearly at this point. It is spread across HaxeResolveUtil.java and HaxeReferenceImpl.java. It currently resolves things out of order (such as a package before a local variable) and has other anomalies. It does not follow the algorithm set out in the Haxe documentation (http://haxe.org/manual/type-system-resolution-order.html).

@soywiz
Copy link
Contributor Author

soywiz commented Jun 3, 2015

Yeah, I know that a lot of people worked on it, and it is a mess because of that. Also I think that projects should start being shit but giving value at the very beginning and then improving with time. So don't worry about that :)

I have done a pull request with a small attempt:

It can detect small functions without the typetag that return constants. For example, when listing methods:

class Test {
public function new() {
    this.<caret>
}
public function test1() { return 10; }
}

That should display the list of methods and test1 should say that it returns an integer. It is a very small work and probably very bad, but hey :)

I would like to have something like:

class HaxeProject { List<HaxeClass> getAllClasses(); HaxeClass getClassByQName(name:String); }

Then HaxeClass or HaxeClassModel being a plain class with just simple stuff:

class HaxeClassModel { PsiElement getElement(); List<HaxeMethodModel> getMethods(); List<HaxeMemberModel> getMethods(); }

But it seems that it is mixed with PsiElement and that have some strange stuff and inheritance depending on the ast. So maybe models representing haxe concepts like project, package, class, member, method... with methods to access elements, and those models allowing to access their psielements.

Something like this:

A container for all types/project:
https://github.com/soywiz/hxlanguageservices/blob/master/src/haxe/languageservices/type/HaxeTypes.hx

A package that allows to query all types in that package:
https://github.com/soywiz/hxlanguageservices/blob/master/src/haxe/languageservices/type/HaxePackage.hx

And so on.
Is there an API like that? If it is, where? If not, could we write that?

@soywiz
Copy link
Contributor Author

soywiz commented Jun 3, 2015

In order to be able to represent fine return types I need the haxe class + specific types. Is there any entity like HaxeSpecificType? That includes the type + all the specific types?

@as3boyan
Copy link
Contributor

as3boyan commented Jun 4, 2015

var a = this.method();

Some time ago I tried to implement very basic inference features, but it's mostly for basic stuff.

@as3boyan
Copy link
Contributor

as3boyan commented Jun 4, 2015

var a = this.method(); // where is resolved the a type obtained from the method? So I can updated that code too.

As Eric said it resolves to classes, but we can grab return type from function I think

https://github.com/TiVo/intellij-haxe/blob/ImplementNestedFunctionTypesSupport/src/common/com/intellij/plugins/haxe/lang/psi/impl/HaxeReferenceImpl.java#L225-225

Maybe this would help

There is NO code that attempts to analyze the PSI to determine the type of a Dynamic variable at any given point.

I would say it's pretty limited #26 + few cases more I think

@EBatTiVo
Copy link
Contributor

EBatTiVo commented Jun 5, 2015

There is code that defines the different types in PsiType.java. Haxe only adds the Dynamic type in HaxePsiTypeAdapter.

A while back, we wanted to incorporate the type hierarchy dialogs and use the existing java code to do that. In so doing, we had to "mixin" a very large number of the HaxeXXX classes with (in most cases) identical java functionality. Fedor had already done some of that work: that's how simple refactoring and a number of other features were implemented. So, we took that concept and ran with it. It ended up that we had to implement a LOT of code to make things compatible. And, since we were using the java classes as our base PSI, there were a number of knotty issues that we eventually worked out. So, there's the history and the reason that we have the class hierarchy we do. If I had (get) to do it all over again, things would be different. That said, as it sits now, a bunch of "Ultimate" functionality comes along for free. The whole class hierarchy display, and the diagrams functionality works, for example.

That said, there's no reason why we can't add new model classes that are conceptually easier to use. We just can't rip out what's there and rework everything. The code must remain compliant to the Java PSI for the code browsing features we want.

A package that allows to query all types in that package: [...] Is there an API like that? If it is, where? If not, could we write that?

Yes, that exists by reading the package name, resolving that, and then walking the PSI tree for the files to find all classes. (The PSI is extended up through the file system, so that the parent of a haxe file is its directory, grandparents are parent directories, and so on. So, once we find the package root, we can get all class-type children using UsefulPsiUtils.findChildren(packageRooot, HaxeClass.class).

@EBatTiVo
Copy link
Contributor

EBatTiVo commented Jun 5, 2015

@soywiz,

So, I've been thinking about what you are after with this issue and #291: Early error detection and display. (Let me know if I'm missing the whole point here...) The haxe compiler already does this, correct? So, instead of re-inventing the wheel, why don't we use it? Something like this:

When a file loads, or after the user hasn't typed anything for a given amount of time (2 seconds?), we call the compiler and ask it to compile the file. We grab the error output (if any) and use the annotator to highlight the error line, and place the error itself in the tooltip.

So, that's the conceptual model. The implementation would look more like:
At startup:

  • Start the compiler server. (It's likely that we will also need some sort of serialization so that we're not calling the compiler for completion at the same time as we are compiling for errors.)

Every time the annotator is called:

  • Check the time.
  • Save the current time.
  • If more than n milliseconds has passed, call the compiler.
  • Once the compiler returns, parse the errors and highlight them.

@soywiz
Copy link
Contributor Author

soywiz commented Jun 5, 2015

Uhm. Compiler doesn't detect all errors. Sometimes it display one error, other time just two errors, or other time errors that are not errors, but a fail in other file.

It is possible to do what you say. But if we are trying to not reinventing the wheel, probably we should just try to use a ocaml2java or ocaml2js source and use it instead, or just use another IDE that uses the compiler fully.

Also we wouldn't be able to provide fixers easily.

I just proposed sometime ago, that they renamed the completion server, to a language services that allowed to performs no just querying stuff, but updating stuff like fixing imports, renaming files and identifiers, and so on. So any IDE could benefit from that. And then all the refactoring code here would be automatically deprecated. But not sure how ended that.

HaxeFoundation/haxe#3275

It was closed. They just considered finding references, but not all the updating stuff.

I ended creating this project: https://github.com/soywiz/hxlanguageservices
But since I'm using intelliJ for creating it, and was a bit painful because no semantic errors were reported at editing time I ended decided helping here instead. And probably using intelliJ is what my company need.

So yeah, we here are inventing a whole wheel. intelliJ itself is probably reinventing a whole wheel because java code doesn't provide services like typescript or C# roslyn, so they had to write all that even when they could just call the java compiler, parse the output and put errors on the screen.

I had to decide which language to use, I could have used typescript (which has already good language tooling), but just compiling to javascript. And use/develop a transpiler to haxe. But I wanted consistency between targets and probably and not that many layers. Also typescript is less type safe than haxe in some aspects. But have some kickass features to have DRY code like defining fields in the constructor declaration as scala does.

I think that if you want to just use an IDE for using the completion server you can just use sublime text.

I like the idea of having a single point of truth and being the compiler a language services instead. And just provide a syntax highlighter and use the compiler (and would be great if the compiler would run in java or javascript directly). But it is not the case at this point. Because the language creators are not providing the tooling that a language needs to be widely used.

@as3boyan
Copy link
Contributor

as3boyan commented Jun 5, 2015

@EBatTiVo I thought about it, about such continuous compilation, when I was working on HIDE, I wanted to compile often to get check for errors do something like live reloading.

but keep in mind if file is not used during compilation - it won't detect any errors, but idea itself is good actually, except that response would be slow and CPU intensive and sometimes limited(not all files compiled)

Maybe it would be nice to have it,

I see why IntelliJ did it, response is very fast, but the thing is that requires lot of work

typescript or C# roslyn

Yeah, seems like they take care of such things, it took them a long time to implement refactoring for those languages, but they got implemented some advanced things to extend IDE.

@EBatTiVo
Copy link
Contributor

EBatTiVo commented Jun 5, 2015

OK. Using the OCAML code directly is an option that I've been considering. Using the compilation server gives us nice code separation, though. The compiler still needs some work either way. I've got a proposal over to Simon to look at.

Part of the reason that the compiler stops after only one or two errors has to do with the way OCAML wants to work. The OCAML language design (to me, having only a couple of weeks' experience with it) seems to be that once you find an error, you just abort the program. Maybe that's not fair, but it seems close. I think that there may be ways to work around it.

Recently, the types of things that can be queried through the completion interface has expanded and they will make some of our job simpler. http://haxe.org/manual/cr-completion.html

@soywiz
Copy link
Contributor Author

soywiz commented Jun 18, 2015

This is already partially done. Obviously doesn't work on all the cases, but this is too generic. So I would close it.

@soywiz soywiz closed this as completed Jun 18, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants