(GRAM, PSI, FMT): introduce foreign_mod_body and mod_body

[mkaput]

No description provided.

[mkaput]

The overall problem here is, that file_mod_item extends mod_item but the former doesn't have anything like a mod_body. I decided to unify their apis (so users do not have to differentiate between mod and file mod) by creating artificial getItemList method on mod_item.

[matklad]

I am not happy about this fileModItem situation at all... It also causes #354.

Maybe we can make RustFile and RustModItem to extend some kind of RustMod?

[mkaput]

Seems reasonable. Implement this here?

[matklad]

Implement this here?

Definitely not here, I predict a massive fallout after this change, especially in the resolve area. It would be cool if you implement this, but I am not sure about the design, so expect quite a bit of back and forth. If you don't want to mess with it, I think I can fix this tomorrow.

I'll be glad to accept foreign_mod part right away!

[mkaput]

I think you can take on this refactoring. Also the mod_item part works, this method will never return null; it's just GK which puts @Nullable there. I could replace orEmpty() with !! and there would be no harm.

[matklad]

Ok, I really hope that I have not underestimate the amount of work the refactoring :)

[alexeykudinkin]

Are there any reasons for the new PSI element (mod_body) introduction?

[matklad]

@alexeykudinkin yep: #350 (comment).

[alexeykudinkin]

@matklad no, it's bad idea to fix indenter and expand-selection by introduction of the new PSI node.

Can hardly spot any other reason for it to be introduced.

[matklad]

@alexeykudinkin and what are the benefits of not introducing the PSI node? I don't feel that either way is significantly better, but simpler formatter is a nice to have.

[alexeykudinkin]

@alexeykudinkin and what are the benefits of not introducing the PSI node?

Mitigating introduction of the useless abstraction is the greatest benefit. The problem is with formatter & expand-selection not with the node.

[matklad]

Sounds reasonable, @jajakobyly what do you think?

[matklad]

I've looked at Java and Kotlin implementations, and the interesting pattern is that { stuff } has its own PsiNode iff it is optional.

That is, in Java PsiClass always has { }, so there is no PsiClassBody. In Kotlin {} is optional, so there are KtClass and KtClassBody.

I think this is a reasonable approach. Based on this, in Rust there should be separate body elements for structs, enums and methods. Mods should not have a separate ModBody because module without a body is a mod_decl.

[mkaput]

Your arguments sound reasonable for me too, though I prefer introducing separate PSI nodes for such things as mod_body as I perceive them as hmm distinguishable entities.

Formatter can be easily adopted to the lack of mod_body, expand selection unfortunately is another story, as I cannot see any abilities to configure it (ExpandSelectionAction) (and it would be actually nice to have an ability to expand selection to the { ... } range, because of already mentioned mod_decl. Though I understand that this is very minor feature.

So should I narrow this PR to foreign_mod_body, or close it completely?

[matklad]

So should I narrow this PR to foreign_mod_body, or close it completely?

foreign_mod_body is also mandatory, so let's close this.