Skip to content

Commit

Permalink
feat: Add no-boolean-literal-for-arguments rule (#1271)
Browse files Browse the repository at this point in the history
* feat(src/rules): Adding the no_boolean_literal_for_arguments.rs file that contains the rule in question and its respective implementation. The newly added rule is now imported in the rules.rs file.

* feat(src/rules,docs): New test cases were added to the no_boolean_literal_for_arguments.rs file. The documentation for the no_boolean_literal_for_arguments rule was introduced in no_boolean_literal_for_arguments.md.

* feat(src/rules,www/static): The new rule was successfully integrated into the rules.rs file. A new docs.json file was generated based on what's specified on the README.md of the project.

* fix(src/rules,www/static,docs/rules): The rule was improved in order to consider every amount of boolean literal arguments as an improvement point. The docs.json file was regenerated. The no_boolean_literal_for_arguments.md was improved to match the changes of the rule.

* fix(project): The code was formatted using the 'deno run --allow-run tools/format.ts' command.

* fix(rules/no_boolean_literal_for_arguments): Adding a simple fix as specified by clippy.

* fix(no_boolean_literal_for_arguments): Code was formatted using the appropriate command.

* fix: windows line endings

---------

Co-authored-by: Marvin Hagemeister <[email protected]>
  • Loading branch information
jmj0502 and marvinhagemeister authored May 8, 2024
1 parent 79bf1b9 commit df778aa
Show file tree
Hide file tree
Showing 4 changed files with 175 additions and 0 deletions.
51 changes: 51 additions & 0 deletions docs/rules/no_boolean_literal_for_arguments.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
Requires all functions called with any amount of `boolean` literals as
parameters to use a self-documenting constant instead.

Is common to define functions that can take `booleans` as arguments. However,
passing `boolean` literals as parameters can lead to lack of context regarding
the role of the argument inside the function in question.

A simple fix for the points mentioned above is the use of self documenting
constants that will end up working as "named booleans", that allow for a better
understanding on what the parameters mean in the context of the function call.

### Invalid

```typescript
function redraw(allViews: boolean, inline: boolean) {
// redraw logic.
}
redraw(true, true);

function executeCommand(recursive: boolean, executionMode: EXECUTION_MODES) {
// executeCommand logic.
}
executeCommand(true, EXECUTION_MODES.ONE);

function enableLogs(enable: boolean) {
// enabledLogs logic.
}
enableLogs(true);
```

### Valid

```typescript
function redraw(allViews: boolean, inline: boolean) {
// redraw logic.
}
const ALL_VIEWS = true, INLINE = true;
redraw(ALL_VIEWS, INLINE);

function executeCommand(recursive: boolean, executionMode: EXECUTION_MODES) {
// executeCommand logic.
}
const RECURSIVE = true;
executeCommand(RECURSIVE, EXECUTION_MODES.ONE);

function enableLogs(enable: boolean) {
// enabledLogs logic.
}
const ENABLE = true;
enableLogs(ENABLE);
```
2 changes: 2 additions & 0 deletions src/rules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ pub mod no_array_constructor;
pub mod no_async_promise_executor;
pub mod no_await_in_loop;
pub mod no_await_in_sync_fn;
pub mod no_boolean_literal_for_arguments;
pub mod no_case_declarations;
pub mod no_class_assign;
pub mod no_compare_neg_zero;
Expand Down Expand Up @@ -252,6 +253,7 @@ fn get_all_rules_raw() -> Vec<&'static dyn LintRule> {
&no_async_promise_executor::NoAsyncPromiseExecutor,
&no_await_in_loop::NoAwaitInLoop,
&no_await_in_sync_fn::NoAwaitInSyncFn,
&no_boolean_literal_for_arguments::NoBooleanLiteralForArguments,
&no_case_declarations::NoCaseDeclarations,
&no_class_assign::NoClassAssign,
&no_compare_neg_zero::NoCompareNegZero,
Expand Down
117 changes: 117 additions & 0 deletions src/rules/no_boolean_literal_for_arguments.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
use super::{Context, LintRule};
use crate::handler::{Handler, Traverse};
use crate::Program;
use deno_ast::view::{CallExpr, NodeTrait};
use deno_ast::SourceRanged;

#[derive(Debug)]
pub struct NoBooleanLiteralForArguments;

const CODE: &str = "no-boolean-literal-for-arguments";
const MESSAGE: &str = "Please create a self-documenting constant instead of \
passing plain booleans values as arguments";
const HINT: &str =
"const ARG_ONE = true, ARG_TWO = false;\nyourFunction(ARG_ONE, ARG_TWO)";

impl LintRule for NoBooleanLiteralForArguments {
fn lint_program_with_ast_view<'view>(
&self,
context: &mut Context<'view>,
program: Program<'view>,
) {
NoBooleanLiteralForArgumentsVisitor.traverse(program, context);
}

fn code(&self) -> &'static str {
CODE
}

fn tags(&self) -> &'static [&'static str] {
&[]
}

#[cfg(feature = "docs")]
fn docs(&self) -> &'static str {
include_str!("../../docs/rules/no_boolean_literal_for_arguments.md")
}
}

struct NoBooleanLiteralForArgumentsVisitor;

impl Handler for NoBooleanLiteralForArgumentsVisitor {
fn call_expr(&mut self, call_expression: &CallExpr, ctx: &mut Context) {
let args = call_expression.args;
let is_boolean_literal =
|text: &str| -> bool { matches!(text, "true" | "false") };
for arg in args {
if is_boolean_literal(arg.text()) {
ctx.add_diagnostic_with_hint(
call_expression.range(),
CODE,
MESSAGE,
HINT,
);
break;
}
}
}
}

#[cfg(test)]
mod test {
use super::*;

#[test]
fn no_boolean_literal_for_arguments_valid() {
assert_lint_ok! {
NoBooleanLiteralForArguments,
r#"runCMDCommand(command, executionMode)"#,
r#"
function formatLog(logData: { level: string, text: string }) {
console.log(`[${level}]:${text}`);
}
formatLog({ level: "INFO", text: "Connected to the DB!" });
"#,
r#"
function displayInformation(display: { renderer: "terminal" | "screen", recursive: boolean }) {
if (display) {
renderInformation();
}
// TODO!
}
displayInformation({ renderer: "terminal", recursive: true });
"#
}
}

#[test]
fn no_boolean_literal_for_arguments_invalid() {
assert_lint_err! {
NoBooleanLiteralForArguments,
r#"test(true,true)"#:[{line: 1, col: 0, message: MESSAGE, hint: HINT}],
r#"test(false,true)"#:[{line: 1, col: 0, message: MESSAGE, hint: HINT}],
r#"test(false,false)"#:[{line: 1, col: 0, message: MESSAGE, hint: HINT}],
r#"invoke(true,remoteServerUrl,true)"#:[{line: 1, col: 0, message: MESSAGE, hint: HINT}],
r#"
function enableLinting(enable: boolean, limitDepth: boolean) {
if (enable) {
linter.run();
}
}
enableLinting(true,false);
"#:[{line: 7, col: 6, message: MESSAGE, hint: HINT}],
r#"
runCMD(true, CMD.MODE_ONE)
"#:[{line: 2, col: 6, message: MESSAGE, hint: HINT}],
r#"
function displayInformation(display: boolean) {
if (display) {
renderInformation();
}
// TODO!
}
displayInformation(true);
"#:[{line: 8, col: 6, message: MESSAGE, hint: HINT}],
}
}
}
5 changes: 5 additions & 0 deletions www/static/docs.json
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,11 @@
"recommended"
]
},
{
"code": "no-boolean-literal-for-arguments",
"docs": "Requires all functions called with any amount of `boolean` literals as\nparameters to use a self-documenting constant instead.\n\nIs common to define functions that can take `booleans` as arguments. However,\npassing `boolean` literals as parameters can lead to lack of context regarding\nthe role of the argument inside the function in question.\n\nA simple fix for the points mentioned above is the use of self documenting\nconstants that will end up working as \"named booleans\", that allow for a better\nunderstanding on what the parameters mean in the context of the function call.\n\n### Invalid\n\n```typescript\nfunction redraw(allViews: boolean, inline: boolean) {\n // redraw logic.\n}\nredraw(true, true);\n\nfunction executeCommand(recursive: boolean, executionMode: EXECUTION_MODES) {\n // executeCommand logic.\n}\nexecuteCommand(true, EXECUTION_MODES.ONE);\n\nfunction enableLogs(enable: boolean) {\n // enabledLogs logic.\n}\nenableLogs(true);\n```\n\n### Valid\n\n```typescript\nfunction redraw(allViews: boolean, inline: boolean) {\n // redraw logic.\n}\nconst ALL_VIEWS = true, INLINE = true;\nredraw(ALL_VIEWS, INLINE);\n\nfunction executeCommand(recursive: boolean, executionMode: EXECUTION_MODES) {\n // executeCommand logic.\n}\nconst RECURSIVE = true;\nexecuteCommand(RECURSIVE, EXECUTION_MODES.ONE);\n\nfunction enableLogs(enable: boolean) {\n // enabledLogs logic.\n}\nconst ENABLE = true;\nenableLogs(ENABLE);\n```\n",
"tags": []
},
{
"code": "no-case-declarations",
"docs": "Requires lexical declarations (`let`, `const`, `function` and `class`) in switch\n`case` or `default` clauses to be scoped with brackets.\n\nWithout brackets in the `case` or `default` block, the lexical declarations are\nvisible to the entire switch block but only get initialized when they are\nassigned, which only happens if that case/default is reached. This can lead to\nunexpected errors. The solution is to ensure each `case` or `default` block is\nwrapped in brackets to scope limit the declarations.\n\n### Invalid:\n\n```typescript\nswitch (choice) {\n // `let`, `const`, `function` and `class` are scoped the entire switch statement here\n case 1:\n let a = \"choice 1\";\n break;\n case 2:\n const b = \"choice 2\";\n break;\n case 3:\n function f() {\n return \"choice 3\";\n }\n break;\n default:\n class C {}\n}\n```\n\n### Valid:\n\n```typescript\nswitch (choice) {\n // The following `case` and `default` clauses are wrapped into blocks using brackets\n case 1: {\n let a = \"choice 1\";\n break;\n }\n case 2: {\n const b = \"choice 2\";\n break;\n }\n case 3: {\n function f() {\n return \"choice 3\";\n }\n break;\n }\n default: {\n class C {}\n }\n}\n```\n",
Expand Down

0 comments on commit df778aa

Please sign in to comment.