Skip to content

Commit

Permalink
Add mixed table lint (Kampfkarren#535)
Browse files Browse the repository at this point in the history
Closes Kampfkarren#534.

This might false positive when working with an external library that
expects a mixed table as it also warns against mixed tables passed into
function calls. Disabling it in these cases would also disable similar
cases where the user's application owns the expecting function.
  • Loading branch information
chriscerie authored Sep 12, 2023
1 parent 84ce358 commit 5f6c2be
Show file tree
Hide file tree
Showing 8 changed files with 217 additions and 0 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
- Excludes are now respected for single files.
- Added `no-exclude` cli flag to disable excludes.
- When given in standard library format, additional information now shows up in `incorrect_standard_library_use` missing required parameter errors.
- Added new [`mixed_table` lint](https://kampfkarren.github.io/selene/lints/mixed_table.html), which will warn against mixed tables.

### Fixed
- `string.pack` and `string.unpack` now have proper function signatures in the Lua 5.3 standard library.
Expand Down
1 change: 1 addition & 0 deletions docs/src/SUMMARY.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
- [incorrect_standard_library_use](./lints/incorrect_standard_library_use.md)
- [manual_table_clone](./lints/manual_table_clone.md)
- [mismatched_arg_count](./lints/mismatched_arg_count.md)
- [mixed_table](./lints/mixed_table.md)
- [multiple_statements](./lints/multiple_statements.md)
- [must_use](./lints/must_use.md)
- [parenthese_conditions](./lints/parenthese_conditions.md)
Expand Down
14 changes: 14 additions & 0 deletions docs/src/lints/mixed_table.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
# mixed_table
## What it does
Checks for mixed tables (tables that act as both an array and dictionary).

## Why this is bad
Mixed tables harms readability and are prone to bugs. There is almost always a better alternative.

## Example
```lua
local foo = {
"array field",
bar = "dictionary field",
}
```
1 change: 1 addition & 0 deletions selene-lib/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -312,6 +312,7 @@ use_lints! {
invalid_lint_filter: lints::invalid_lint_filter::InvalidLintFilterLint,
manual_table_clone: lints::manual_table_clone::ManualTableCloneLint,
mismatched_arg_count: lints::mismatched_arg_count::MismatchedArgCountLint,
mixed_table: lints::mixed_table::MixedTableLint,
multiple_statements: lints::multiple_statements::MultipleStatementsLint,
must_use: lints::must_use::MustUseLint,
parenthese_conditions: lints::parenthese_conditions::ParentheseConditionsLint,
Expand Down
1 change: 1 addition & 0 deletions selene-lib/src/lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ pub mod ifs_same_cond;
pub mod invalid_lint_filter;
pub mod manual_table_clone;
pub mod mismatched_arg_count;
pub mod mixed_table;
pub mod multiple_statements;
pub mod must_use;
pub mod parenthese_conditions;
Expand Down
92 changes: 92 additions & 0 deletions selene-lib/src/lints/mixed_table.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
use super::*;
use crate::ast_util::range;
use std::convert::Infallible;

use full_moon::{
ast::{self, Ast},
visitors::Visitor,
};

pub struct MixedTableLint;

impl Lint for MixedTableLint {
type Config = ();
type Error = Infallible;

const SEVERITY: Severity = Severity::Warning;
const LINT_TYPE: LintType = LintType::Correctness;

fn new(_: Self::Config) -> Result<Self, Self::Error> {
Ok(MixedTableLint)
}

fn pass(&self, ast: &Ast, _: &Context, _: &AstContext) -> Vec<Diagnostic> {
let mut visitor = MixedTableVisitor::default();

visitor.visit_ast(ast);

let mut diagnostics = Vec::new();

for mixed_table in visitor.mixed_tables {
diagnostics.push(Diagnostic::new_complete(
"mixed_table",
"mixed tables are not allowed".to_owned(),
Label::new(mixed_table.range),
vec!["help: change this table to either an array or dictionary".to_owned()],
Vec::new(),
));
}

diagnostics
}
}

#[derive(Default)]
struct MixedTableVisitor {
mixed_tables: Vec<MixedTable>,
}

struct MixedTable {
range: (usize, usize),
}

impl Visitor for MixedTableVisitor {
fn visit_table_constructor(&mut self, node: &ast::TableConstructor) {
let mut last_key_field_starting_range = 0;
let mut last_no_key_field_starting_range = 0;

for field in node.fields() {
if let ast::Field::NoKey(_) = field {
if last_key_field_starting_range > 0 {
self.mixed_tables.push(MixedTable {
range: (last_key_field_starting_range, range(field).1),
});
return;
}
last_no_key_field_starting_range = range(field).0;
} else {
if last_no_key_field_starting_range > 0 {
self.mixed_tables.push(MixedTable {
range: (last_no_key_field_starting_range, range(field).1),
});
return;
}
last_key_field_starting_range = range(field).0;
}
}
}
}

#[cfg(test)]
mod tests {
use super::{super::test_util::test_lint, *};

#[test]
fn test_mixed_table() {
test_lint(
MixedTableLint::new(()).unwrap(),
"mixed_table",
"mixed_table",
);
}
}
53 changes: 53 additions & 0 deletions selene-lib/tests/lints/mixed_table/mixed_table.lua
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
local bad = {
"",
a = b,
}

bad = {
{},
[a] = b,
}

bad = {
a,
[""] = b,
}

-- This is technically not a mixed table, but it's formatted like it harming readability
-- so it should still be linted
bad = {
1,
[2] = b,
}

bad = {
[a] = b,
[c] = d,
"",
}

bad({
a = b,
"",
c = d,
})

local good = {
a = b,
c = d,
}

good = {
"",
a,
}

good = {
[1] = a,
[3] = b,
}

good({
a = b,
c = d,
})
54 changes: 54 additions & 0 deletions selene-lib/tests/lints/mixed_table/mixed_table.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
error[mixed_table]: mixed tables are not allowed
┌─ mixed_table.lua:2:5
2 │ ╭ "",
3 │ │ a = b,
│ ╰─────────^
= help: change this table to either an array or dictionary

error[mixed_table]: mixed tables are not allowed
┌─ mixed_table.lua:7:5
7 │ ╭ {},
8 │ │ [a] = b,
│ ╰───────────^
= help: change this table to either an array or dictionary

error[mixed_table]: mixed tables are not allowed
┌─ mixed_table.lua:12:5
12 │ ╭ a,
13 │ │ [""] = b,
│ ╰────────────^
= help: change this table to either an array or dictionary

error[mixed_table]: mixed tables are not allowed
┌─ mixed_table.lua:19:5
19 │ ╭ 1,
20 │ │ [2] = b,
│ ╰───────────^
= help: change this table to either an array or dictionary

error[mixed_table]: mixed tables are not allowed
┌─ mixed_table.lua:25:5
25 │ ╭ [c] = d,
26 │ │ "",
│ ╰──────^
= help: change this table to either an array or dictionary

error[mixed_table]: mixed tables are not allowed
┌─ mixed_table.lua:30:5
30 │ ╭ a = b,
31 │ │ "",
│ ╰──────^
= help: change this table to either an array or dictionary

0 comments on commit 5f6c2be

Please sign in to comment.