-
-
Notifications
You must be signed in to change notification settings - Fork 354
Validation and enforcement of @class
fields is incomplete/inconsistent
#1823
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
Comments
This should probably be broken into multiple separate issues with some overlap: Exact type checking (all examples) I'd like this too, but I'm not sure if the extension should check for missing fields by default, as that would probably require a lot of changes to existing code bases that use types in a more flexible way. This is particularly true for examples 4 to 6, where passing/returning arrays and non-annotated tables is allowed even if the function is annotated to accept/return specific types. Perhaps a new annotation when defining classes like my suggestion here #1602 Alternatively (or in addition), a new annotation could be introduced to do the checking when using the type (e.g. Generic This seems like a more generic problem with the definition of Type of return statement (example 5) There's no way to specify the type of a table returned by a return statement other than creating a typed local variable. It's also weird that even basic checking for invalid type isn't done in this case. I think a fix for this would be good even if the other examples with exact types aren't implemented. It also comes up with modules: #1656 |
Thanks for looking into it. 💛
I agree, both the annotation for final/exact classes and the annotation for exact return values make a lot of sense. Treating classes as final by default would indeed be a huge breaking change that should be avoided. But to be honest I was very surprised when I saw that invalid or missing fields are not checked with the current implementation.
You are right, that seems to be the case. Now it makes sense to me why these examples don't work. What I would expect is that lua-language-server traces back the annotated type to all places where the variable is modified. After all, that's also what happens when I declare more simple param types. So if I say that a method takes an array of a particular type, I'd expect that the passed variable is internally marked to be that. Then everything that gets added to the table must be of the declared type.
Do I understand you correctly that the type of a Similar to param types, I'd expect lua-language-server to trace the type back from the return statement. |
I'm not familiar with the internals of the extension or how it traces variable types. My experience is that it usually works forward from annotations, and not backwards. What you propose does makes sense, if it's possible.
It's not ignored. The basic type is checked (e.g. returning a number when you used Since there's no way to explicitly specify the type of a directly returned table ( |
You can add (exact) classes being treated as having a __call metamethod even when one isn't defined to the list of odd behaviors. |
How are you using the lua-language-server?
Visual Studio Code Extension (sumneko.lua)
Which OS are you using?
MacOS
What is the issue affecting?
Annotations, Type Checking
Expected Behaviour
When using a
@class
type in a type hint, lua-language-server should ensure the following code properties:All of these code properties should be checked when a
@param
or@return
type hint specifies its type to be a class or an array of classes.Actual Behaviour
When passing a class as an argument, only wrong field types are flagged. The other properties are not checked.
When passing an array of classes as an argument, none of the properties are checked. The same applies to returning a class or array of classes.
Reproduction steps
Try the following code. Only the first example is correctly flagged. The remaining examples produce no diagnostics whatsoever.
Additional Notes
No response
Log File
The text was updated successfully, but these errors were encountered: