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

Adds table field checks #65

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
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
28 changes: 16 additions & 12 deletions spec/cli_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -296,32 +296,36 @@ Total: 5 warnings / 0 errors in 1 file
end)

it("raises critical errors on config without additional operators", function()
assert.equal([[Checking spec/samples/compound_operators.lua 4 errors
assert.equal([[Checking spec/samples/compound_operators.lua 1 warning / 4 errors

spec/samples/compound_operators.lua:2:1: assignment uses compound operator +=
spec/samples/compound_operators.lua:3:1: assignment uses compound operator -=
spec/samples/compound_operators.lua:5:2: assignment uses compound operator /=
spec/samples/compound_operators.lua:9:3: value assigned to table field 't'.'a' is unused
spec/samples/compound_operators.lua:10:1: assignment uses compound operator *=

Total: 0 warnings / 4 errors in 1 file
Total: 1 warning / 4 errors in 1 file
]], get_output "spec/samples/compound_operators.lua --no-config")
end)

it("raises critical errors for unfiltered additional operators", function()
assert.equal([[Checking spec/samples/compound_operators.lua 3 errors
assert.equal([[Checking spec/samples/compound_operators.lua 1 warning / 3 errors

spec/samples/compound_operators.lua:3:1: assignment uses compound operator -=
spec/samples/compound_operators.lua:5:2: assignment uses compound operator /=
spec/samples/compound_operators.lua:9:3: value assigned to table field 't'.'a' is unused
spec/samples/compound_operators.lua:10:1: assignment uses compound operator *=

Total: 0 warnings / 3 errors in 1 file
Total: 1 warning / 3 errors in 1 file
]], get_output "spec/samples/compound_operators.lua --no-config --operators +=")
end)

it("allows to define allowed compound operators", function()
assert.equal([[Checking spec/samples/compound_operators.lua OK
assert.equal([[Checking spec/samples/compound_operators.lua 1 warning

Total: 0 warnings / 0 errors in 1 file
spec/samples/compound_operators.lua:9:3: value assigned to table field 't'.'a' is unused

Total: 1 warning / 0 errors in 1 file
]], get_output "spec/samples/compound_operators.lua --config=spec/configs/compound_operators_config.luacheckrc")
end)

Expand Down Expand Up @@ -1236,7 +1240,7 @@ Codes: true
assert.equal(([[
Checking spec/samples/argparse-0.2.0.lua 9 warnings
Checking spec/samples/compat.lua 4 warnings
Checking spec/samples/compound_operators.lua 4 errors
Checking spec/samples/compound_operators.lua 1 warning / 4 errors
Checking spec/samples/custom_std_inline_options.lua 3 warnings / 1 error
Checking spec/samples/global_inline_options.lua 3 warnings
Checking spec/samples/globals.lua 2 warnings
Expand All @@ -1254,7 +1258,7 @@ Checking spec/samples/unused_secondaries.lua 4 warnings
Checking spec/samples/utf8.lua 5 warnings
Checking spec/samples/utf8_error.lua 1 error

Total: 74 warnings / 5 errors in 19 files
Total: 77 warnings / 9 errors in 21 files
]]):gsub("(spec/samples)/", "%1"..package.config:sub(1, 1)),
get_output "spec/samples --config=spec/configs/exclude_files_config.luacheckrc -qq --exclude-files spec/samples/global_fields.lua")
end)
Expand All @@ -1263,7 +1267,7 @@ Total: 74 warnings / 5 errors in 19 files
assert.equal([[
Checking argparse-0.2.0.lua 9 warnings
Checking compat.lua 4 warnings
Checking compound_operators.lua 4 errors
Checking compound_operators.lua 1 warning / 4 errors
Checking custom_std_inline_options.lua 3 warnings / 1 error
Checking global_inline_options.lua 3 warnings
Checking globals.lua 2 warnings
Expand All @@ -1281,15 +1285,15 @@ Checking unused_secondaries.lua 4 warnings
Checking utf8.lua 5 warnings
Checking utf8_error.lua 1 error

Total: 74 warnings / 5 errors in 19 files
Total: 77 warnings / 9 errors in 21 files
]], get_output(". --config=spec/configs/exclude_files_config.luacheckrc -qq --exclude-files global_fields.lua", "spec/samples/"))
end)

it("combines excluded files from config and cli", function()
assert.equal([[
Checking argparse-0.2.0.lua 9 warnings
Checking compat.lua 4 warnings
Checking compound_operators.lua 4 errors
Checking compound_operators.lua 1 warning / 4 errors
Checking custom_std_inline_options.lua 3 warnings / 1 error
Checking global_inline_options.lua 3 warnings
Checking globals.lua 2 warnings
Expand All @@ -1305,7 +1309,7 @@ Checking unused_secondaries.lua 4 warnings
Checking utf8.lua 5 warnings
Checking utf8_error.lua 1 error

Total: 66 warnings / 5 errors in 17 files
Total: 69 warnings / 9 errors in 19 files
]], get_output(". --config=spec/configs/exclude_files_config.luacheckrc -qq --exclude-files global_fields.lua --exclude-files " .. quote("./read*"), "spec/samples/"))
end)

Expand Down
3 changes: 2 additions & 1 deletion src/luacheck/stages/check_table_fields.lua
Original file line number Diff line number Diff line change
Expand Up @@ -437,7 +437,8 @@ local item_callbacks = {
Cjump = ClosureState.handle_jump,
Eval = ClosureState.handle_eval,
Local = ClosureState.handle_local_or_set_item,
Set = ClosureState.handle_local_or_set_item
Set = ClosureState.handle_local_or_set_item,
OpSet = ClosureState.handle_local_or_set_item,
Copy link
Member

Choose a reason for hiding this comment

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

@arichard4 I took a stab at both fixing the Lua error and integrating the test result changes from the last few releases into this branch. I wasn't 100% sure about this fix though, does it look sane to you?

Copy link
Author

Choose a reason for hiding this comment

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

Sorry, missed this earlier.

No, unfortunately that's not right- I'll need to write a different handler for the OpSet case. The problem is that an OpSet is both a read and a write, while this only handles a write- the Set case. For a concrete example, the compound operators test produces this incorrect output:

t.a = i --value assigned to table field 't'.'a' is unused
t.a *= 2 -- because the existing code treats this as only an overwriting Set to t.a, not as also reading the previous value of t.a
return t```

Copy link
Member

Choose a reason for hiding this comment

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

Fair enough. Feel free to reset to before my merge or rewrite/amend the merge to correct this or just add a new commit to adjust as appropriate.

Copy link
Author

Choose a reason for hiding this comment

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

Pushed a revised version

}

-- Steps through the closure one item at a time
Expand Down
Loading
You are viewing a condensed version of this merge commit. You can view the full changes here.