Benjamin-Dobell / IntelliJ-Luanalysis

Type-safe Lua IDE — IntelliJ IDEA plugin
Apache License 2.0
154 stars 21 forks source link

ILuaTypeInfer does only call first found extension instead of all #110

Open knoxfighter opened 2 years ago

knoxfighter commented 2 years ago

Environment

Name Version
IDEA version IU2021.2.3
Luanalysis version 1.3.0
OS Ubuntu 18.04

What happens?

I want to add additional type guessing for the Factorio LUA API plugin i write, while moving it from EmmLUA to Luanalysis. I run into the problem, that i can only override all type guessing, which causes some sideeffects like wrongly guessed/created classes. It would be great to have the possibility to add typeGuesser without overriding the default one (like in EmmyLua itself).

Current code you use to guess a type:

fun infer(target: LuaTypeGuessable, context: SearchContext): ITy? {
    for (typeInfer in EP_NAME.extensionList) {
        ProgressManager.checkCanceled()
        return typeInfer.inferType(target, context)
    }
    return null
}

I would suggest to call all added extensions. (I would use type any to determine if nothing useful was found.

Like this:

fun infer(target: LuaTypeGuessable, context: SearchContext): ITy? {
    for (typeInfer in EP_NAME.extensionList) {
        ProgressManager.checkCanceled()
        val type = typeInfer.inferType(target, context)
        if (type != null) {
            if (type != Ty.getBuiltin(Constants.WORD_ANY)) {
                return type
            }
        }
    }
    return null
}

really rough code, since i am not used to kotlin or Luanalysis internals.

Benjamin-Dobell commented 2 years ago

Admittedly, I hadn't thought about this at all. I've been going wild changing the internals with very little thought regarding third-party extension. However, what you're after certainly sounds reasonable to me.

knoxfighter commented 2 years ago

If you want, you can also redesign the inferType function a little bit. That would spare me a second call to inferInner. That would make it possible to run exactly after your LuaTypeInfer implementation and to use your guessed Type to override it.

// ILuaTypeInfer.kt
interface ILuaTypeInfer {
    companion object {
        private val EP_NAME = ExtensionPointName.create<ILuaTypeInfer>("au.com.glassechidna.luanalysis.luaTypeInfer")

        fun infer(target: LuaTypeGuessable, context: SearchContext): ITy? {
            var iTy : ITy? = null
            for (typeInfer in EP_NAME.extensionList) {
                ProgressManager.checkCanceled()
                iTy = typeInfer.inferType(target, context, iTy)
            }
            return iTy
        }
    }

    fun inferType(target: LuaTypeGuessable, context: SearchContext, iTy: ITy?): ITy?
}

// SpecializedTypeInfer.java
@Nullable
@Override
public ITy inferType(@NotNull LuaTypeGuessable luaTypeGuessable, @NotNull SearchContext searchContext, @Nullable ITy iTy) {
    // do further type guessing and override if needed

    // return new found or old type
    return iTy;
}
knoxfighter commented 2 years ago

Since that is blocking me: Which way do you want it to be implemented? I will try to add that feature back in myself, so i can get that problem out of my mind :)

Benjamin-Dobell commented 2 years ago

If you submit a PR along the lines of your original idea, that'd be preferable for now. However, stick to null comparisons rather than using any to signify the lack of inference[1].

I'm not entirely adverse to changing the external API, however I'll have to think about the semantics some more. It may, for example, be important to know which extension inferred the "current" type.

[1] EmmyLua used any to signify the lack of a type internally and I've made a conscious effort to move away from this, although the effort is incomplete. The reasoning is that I want the lack of type inference to bubble as some of the more advanced type checking logic needs to be able to discern the lack of inference from explicitly typed any.

Also, at some point in the future I'd like to make the "lack of inference" behaviour user configurable. Specifically, I'd like to introduce a unknown type similar to that found in TypeScript; a type that exhibits safer variance semantics.