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

Support for Operator type annotations #599

Closed
Stanzilla opened this issue Jul 14, 2021 · 18 comments
Closed

Support for Operator type annotations #599

Stanzilla opened this issue Jul 14, 2021 · 18 comments
Labels
enhancement New feature or request priority.low A low priority topic
Milestone

Comments

@Stanzilla
Copy link

It does not look like Emmy does have native support for Operator type annotations but I was wondering if it was possible to add it to the LSP?

@sumneko sumneko added enhancement New feature or request priority.low A low priority topic labels Jul 15, 2021
@sumneko
Copy link
Collaborator

sumneko commented Jul 15, 2021

Considering performance issues, a new annotation should be provided for explicit marking.

@Stanzilla
Copy link
Author

@sumneko but that is still something you can do inside of the LSP, right?

@sumneko
Copy link
Collaborator

sumneko commented Jul 16, 2021

My current assumption is this:

---@class A
---@operator add string

---@type A
local a
local r = a + 1 -- `r` is `string` here

For performance reasons, you must define this explicitly, otherwise the cost performance from metatable search is too low.

@Benjamin-Dobell
Copy link

Benjamin-Dobell commented Jul 17, 2021

May I please instead suggest:

unary_op ::= 'unm'
binary_op ::= 'add' | 'sub' | 'mul' | 'div' | 'idiv' | 'mod' | 'pow' | 'concat'
unary_operator ::= unary_op ':'? ty
binary_operator ::= binary_op '(' ty ')' ':' ty
tag_operator ::= 'operator' (unary_operator | binary_operator) comment_string?

In order to cover:

---@class A
---@operator unm: A
---@operator add(A): A
---@operator add(number): number

---@type A
local a1

---@type A
local a2

local foo = a1 + a2 -- A
local bar = a1 + 1 -- number
local huzzah = -a1 -- A

@sumneko
Copy link
Collaborator

sumneko commented Jul 19, 2021

@Benjamin-Dobell This kind of generic support may bring a relatively large overhead, I don’t want to support it for the time being

@Benjamin-Dobell
Copy link

Benjamin-Dobell commented Jul 19, 2021

Sorry, just to to clarify, I'm not requesting you add all those operators or that you infer the right operand's type. Instead I'd like to try come up with a syntax that is at least future proof with respect to types, in order to avoid the pitfalls of LDoc.

I'm the developer of Luanalysis. Luanalysis does not yet have this functionality but, when implemented, will use the right operand's type when inferring the resultant type of the expression. Even if you opt to ignore the right operand's type, it'd be ideal if we supported the same syntax.

@sumneko
Copy link
Collaborator

sumneko commented Jul 19, 2021

Sorry, just to to clarify, I'm not requesting you add all those operators. Instead I'd like to try come up with a syntax that is future proof with respect to types, in order to avoid the pitfalls of LDoc.

I'm the developer of Luanalysis. Luanalysis does not yet have this functionality but, when implemented, will use the right operand's type when inferring the resultant type of the expression. Even if you opt to ignore the right operand's type, it'd be ideal if we supported the same syntax.

It looks good, and I will follow this format to implement it later (including compatibility and ignoring the type of operation object).

@Stanzilla
Copy link
Author

@sumneko any news? :)

@sumneko
Copy link
Collaborator

sumneko commented Jun 23, 2022

Will support in 3.5.0

@sumneko
Copy link
Collaborator

sumneko commented Jul 6, 2022

Done.

@sumneko sumneko closed this as completed Jul 6, 2022
@nathanpage-credo
Copy link

@sumneko Will this also support extending current classes/types. For example, I've used penlight stringx.format_operator which allows "%d %d" % {2,3}. This shows up as a param-type-mismatch.

@sumneko
Copy link
Collaborator

sumneko commented Jul 7, 2022

@sumneko Will this also support extending current classes/types. For example, I've used penlight stringx.format_operator which allows "%d %d" % {2,3}. This shows up as a param-type-mismatch.

Please open a new issue and tell more details.

@BribeFromTheHive
Copy link

How do I document an __index metamethod? I have a table which extends an arbitrary number of sub-elements via OriginalTableName.subElementName syntax. I have no idea how to tell the extension that the subElementNames are of type "blahBlahBlah" instead of it defaulting to "unknown".

@sumneko
Copy link
Collaborator

sumneko commented Nov 21, 2022

How do I document an __index metamethod? I have a table which extends an arbitrary number of sub-elements via OriginalTableName.subElementName syntax. I have no idea how to tell the extension that the subElementNames are of type "blahBlahBlah" instead of it defaulting to "unknown".

---@class Cat: Animal

@CelticMinstrel
Copy link

I have a question about the operator annotation. Is it also required if the operator metafunction is fully declared in the Lua file? Or is that detected separately?

@sumneko
Copy link
Collaborator

sumneko commented Feb 6, 2023

I have a question about the operator annotation. Is it also required if the operator metafunction is fully declared in the Lua file? Or is that detected separately?

Operator annotation is required.

@CelticMinstrel
Copy link

Another question. What about the comparison operators? That is, eq / lt / le. Currently it reports those to be an unknown operator.

@sumneko
Copy link
Collaborator

sumneko commented Feb 6, 2023

Another question. What about the comparison operators? That is, eq / lt / le. Currently it reports those to be an unknown operator.

Not supported yet.
If you have other questions or feature requests, please open a new issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request priority.low A low priority topic
Projects
None yet
Development

No branches or pull requests

6 participants