Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# changelog

## Unreleased
* `NEW` diagnostic: `no-unknown-operations`
<!-- Add all new changes here. They will be moved under a version at release -->

## 3.16.0
Expand Down
4 changes: 4 additions & 0 deletions locale/en-us/script.lua
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,10 @@ DIAG_COUNT_DOWN_LOOP =
'Do you mean `{}` ?'
DIAG_UNKNOWN =
'Can not infer type.'
DIAG_UNKNOWN_OPERATION_CALL =
'Cannot infer call result for type {}.'
DIAG_UNKNOWN_OPERATION_OPERATOR =
'Cannot infer `{}` result for types {} and {}.'
DIAG_DEPRECATED =
'Deprecated.'
DIAG_DIFFERENT_REQUIRES =
Expand Down
2 changes: 2 additions & 0 deletions locale/en-us/setting.lua
Original file line number Diff line number Diff line change
Expand Up @@ -425,6 +425,8 @@ config.diagnostics['unnecessary-assert'] =
'Enable diagnostics for redundant assertions on truthy values.'
config.diagnostics['no-unknown'] =
'Enable diagnostics for cases in which the type cannot be inferred.'
config.diagnostics['no-unknown-operations'] =
'Enable diagnostics for cases in which the result type of an operation cannot be inferred.'
config.diagnostics['not-yieldable'] =
'Enable diagnostics for calls to `coroutine.yield()` when it is not permitted.'
config.diagnostics['param-type-mismatch'] =
Expand Down
58 changes: 58 additions & 0 deletions script/core/diagnostics/no-unknown-operations.lua
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
local files = require 'files'
local guide = require 'parser.guide'
local lang = require 'language'
local vm = require 'vm'
local await = require 'await'

---@async
return function (uri, callback)
local state = files.getState(uri)
if not state then return end
-- TODO: no-unknown doesn't do this but missing-local-export-doc does, is this actually needed?
if not state.ast then return end
Comment on lines +11 to +12

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The TODO here questions the necessity of the if not state.ast then return end check. It's a good practice to keep this check for robustness. It acts as a safeguard against potential crashes if files.getState(uri) returns a state object that, for some reason (like a severe parsing error), lacks an AST. Keeping this check makes the diagnostic more resilient. I'd recommend removing the TODO comment.


-- calls are complicated because unknown arguments may or may not cause an introduction of an unknown type
-- integer(unknown) :: unknown should count as introducing an unknown, but function(unknown) :: unknown should not. We can't directly
-- check function because it might be overloaded or have a call operator defined.
---@async
guide.eachSourceType(state.ast, 'call', function (source)
await.delay()

local resultInfer = vm.getInfer(source):view(uri)
if resultInfer ~= 'unknown' then return end
local functionType = vm.getInfer(source.node)
if functionType:view(uri) == 'unknown' then return end -- we can't say anything about what unknown types support
if functionType:isCallable(uri) then return end
callback {
start = source.start,
finish = source.finish,
message = lang.script('DIAG_UNKNOWN_OPERATION_CALL', functionType:view(uri)),
}
end)

-- binary operators are quite similar to function calls, they introduce an unknown if the result is unknown and none of the
-- parameters are unknown, or if the left side is known to not implement the operator

---@async
guide.eachSourceType(state.ast, 'binary', function (source)
await.delay()

local resultInfer = vm.getInfer(source)
if resultInfer:view(uri) ~= 'unknown' then return end
local left, right = source[1], source[2]
local leftInfer, rightInfer = vm.getInfer(left), vm.getInfer(right)
if leftInfer:view(uri) == 'unknown' then return end
if rightInfer:view(uri) ~= 'unknown' then
-- the operator doesn't work for these types
callback {
start = source.start,
finish = source.finish,
message = lang.script('DIAG_UNKNOWN_OPERATION_OPERATOR', source.op.type, leftInfer:view(uri), rightInfer:view(uri)),
}
return
end

-- TODO: it seems that if the operator is defined and the other arg is unkown it is always inferred as the
-- return type of the operator, so we can't check that case currently
Comment on lines +55 to +56

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The TODO comment discusses a scenario where an operation with a known type and an unknown type results in unknown. The current implementation correctly chooses not to report a diagnostic in this case. This diagnostic should focus on operations that are inherently invalid for the given known types, rather than issues stemming from a variable that is already unknown. The no-unknown diagnostic is better suited for flagging the unknown variable itself. Since the current behavior is correct, this TODO can be removed to avoid confusion.

end)
end
1 change: 1 addition & 0 deletions script/proto/diagnostic.lua
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,7 @@ m.register {

m.register {
'no-unknown',
'no-unknown-operations',
} {
group = 'strong',
severity = 'Warning',
Expand Down
6 changes: 6 additions & 0 deletions script/vm/infer.lua
Original file line number Diff line number Diff line change
Expand Up @@ -383,6 +383,12 @@ function mt:hasFunction(uri)
or self._hasDocFunction == true
end

---@param uri uri
---@return boolean
function mt:isCallable(uri)
return self:hasFunction(uri) or #vm.getOperators("call", self.node, uri) ~= 0
end

---@param uri uri
function mt:_computeViews(uri)
if self.views then
Expand Down
107 changes: 61 additions & 46 deletions script/vm/operator.lua
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
---@class vm
local vm = require 'vm.vm'
local util = require 'utility'
local guide = require 'parser.guide'
local config = require 'config'
local vm = require 'vm.vm'
local util = require 'utility'
local guide = require 'parser.guide'
local config = require 'config'

vm.UNARY_OP = {
vm.UNARY_OP = {
'unm',
'bnot',
'len',
}
vm.BINARY_OP = {
vm.BINARY_OP = {
'add',
'sub',
'mul',
Expand All @@ -24,17 +24,17 @@ vm.BINARY_OP = {
'shr',
'concat',
}
vm.OTHER_OP = {
vm.OTHER_OP = {
'call',
}

local unaryMap = {
local unaryMap = {
['-'] = 'unm',
['~'] = 'bnot',
['#'] = 'len',
}

local binaryMap = {
local binaryMap = {
['+'] = 'add',
['-'] = 'sub',
['*'] = 'mul',
Expand All @@ -50,7 +50,7 @@ local binaryMap = {
['..'] = 'concat',
}

local otherMap = {
local otherMap = {
['()'] = 'call',
}

Expand Down Expand Up @@ -95,13 +95,12 @@ local function checkOperators(operators, op, value, result)
end

---@param op string
---@param exp parser.object
---@param value? parser.object
---@return vm.node?
function vm.runOperator(op, exp, value)
local uri = guide.getUri(exp)
local node = vm.compileNode(exp)
local result
---@param node vm.node
---@param uri string
---@return parser.object[]
function vm.getOperators(op, node, uri)
---@type parser.object[]
local operators = {}
for c in node:eachObject() do
if c.type == 'string'
or c.type == 'doc.type.string' then
Expand All @@ -111,11 +110,27 @@ function vm.runOperator(op, exp, value)
---@cast c vm.global
for _, set in ipairs(c:getSets(uri)) do
if set.operators and #set.operators > 0 then
result = checkOperators(set.operators, op, value, result)
for _, operator in ipairs(set.operators) do
if operator.op[1] == op then
table.insert(operators, operator)
end
end
end
end
end
end
return operators
end

---@param op string
---@param exp parser.object
---@param value? parser.object
---@return vm.node?
function vm.runOperator(op, exp, value)
local node = vm.compileNode(exp)
local uri = guide.getUri(exp)
local operators = vm.getOperators(op, node, uri)
local result = checkOperators(operators, op, value, nil)
return result
end

Expand Down Expand Up @@ -247,10 +262,10 @@ vm.binarySwitch = util.switch()
local op = source.op.type
if a and b then
local result = op == '<<' and a << b
or op == '>>' and a >> b
or op == '&' and a & b
or op == '|' and a | b
or op == '~' and a ~ b
or op == '>>' and a >> b
or op == '&' and a & b
or op == '|' and a | b
or op == '~' and a ~ b
---@diagnostic disable-next-line: missing-fields
vm.setNode(source, {
type = 'integer',
Expand Down Expand Up @@ -281,18 +296,18 @@ vm.binarySwitch = util.switch()
local b = vm.getNumber(source[2])
local op = source.op.type
local zero = b == 0
and ( op == '%'
or op == '/'
or op == '//'
)
and (op == '%'
or op == '/'
or op == '//'
)
if a and b and not zero then
local result = op == '+' and a + b
or op == '-' and a - b
or op == '*' and a * b
or op == '/' and a / b
or op == '%' and a % b
or op == '//' and a // b
or op == '^' and a ^ b
local result = op == '+' and a + b
or op == '-' and a - b
or op == '*' and a * b
or op == '/' and a / b
or op == '%' and a % b
or op == '//' and a // b
or op == '^' and a ^ b
---@diagnostic disable-next-line: missing-fields
vm.setNode(source, {
type = (op == '//' or math.type(result) == 'integer') and 'integer' or 'number',
Expand Down Expand Up @@ -353,10 +368,10 @@ vm.binarySwitch = util.switch()
end)
: case '..'
: call(function (source)
local a = vm.getString(source[1])
or vm.getNumber(source[1])
local b = vm.getString(source[2])
or vm.getNumber(source[2])
local a = vm.getString(source[1])
or vm.getNumber(source[1])
local b = vm.getString(source[2])
or vm.getNumber(source[2])
if a and b then
if type(a) == 'number' or type(b) == 'number' then
local uri = guide.getUri(source)
Expand Down Expand Up @@ -390,13 +405,13 @@ vm.binarySwitch = util.switch()
local infer2 = vm.getInfer(source[2])
if (
infer1:hasType(uri, 'integer')
or infer1:hasType(uri, 'number')
or infer1:hasType(uri, 'string')
or infer1:hasType(uri, 'number')
or infer1:hasType(uri, 'string')
)
and (
infer2:hasType(uri, 'integer')
or infer2:hasType(uri, 'number')
or infer2:hasType(uri, 'string')
or infer2:hasType(uri, 'number')
or infer2:hasType(uri, 'string')
) then
vm.setNode(source, vm.declareGlobal('type', 'string'))
return
Expand All @@ -419,17 +434,17 @@ vm.binarySwitch = util.switch()
local b = vm.getNumber(source[2])
if a and b then
local op = source.op.type
local result = op == '>' and a > b
or op == '<' and a < b
or op == '>=' and a >= b
or op == '<=' and a <= b
local result = op == '>' and a > b
or op == '<' and a < b
or op == '>=' and a >= b
or op == '<=' and a <= b
---@diagnostic disable-next-line: missing-fields
vm.setNode(source, {
type = 'boolean',
start = source.start,
finish = source.finish,
parent = source,
[1] =result,
[1] = result,
})
else
vm.setNode(source, vm.declareGlobal('type', 'boolean'))
Expand Down
1 change: 1 addition & 0 deletions test/diagnostics/init.lua
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ check 'unnecessary-assert'
check 'newfield-call'
check 'newline-call'
check 'not-yieldable'
check 'no-unknown-operations'
check 'param-type-mismatch'
check 'redefined-local'
check 'redundant-parameter'
Expand Down
80 changes: 80 additions & 0 deletions test/diagnostics/no-unknown-operations.lua
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
TEST [[
local x = 5
<!x()!>

---@overload fun(): string
local x = 5
x()

---@class x
---@operator call(): string
local x = {}
x()
]]

TEST [[
---@type nil
local x
<!x()!>

---@overload fun(): string
local x
x()

---@class x
---@operator call(): string
local x
x()
]]

TEST [[
---@type unknown
local x
x()
]]

TEST [[
local function f()
end
f()
]]

TEST [[
<!(0)()!>
]]

TEST [[
<!("")()!>
]]

TEST [[
(function() end)()
]]

TEST [[
local _ = 1 + 2
local _ = <!"1" + "2"!>
]]

TEST [[
local _ = "a" .. "b"
---@class Vec2

---@type Vec2, Vec2
local a, b = {}, {}
local _ = <!a .. b!>
]]

TEST [[
---@type unknown
local a
local b = a + 2
]]

TEST [[
---@param name string
---@return string
local function greet(name)
return "Hello " .. <!name "!"!>
end
]]
Loading