Skip to content

feat: add require-read-r rule - #11

Open
nzakas wants to merge 2 commits into
rule/no-useless-catfrom
rule/require-read-r
Open

nzakas wants to merge 2 commits into
rule/no-useless-catfrom
rule/require-read-r

Conversation

@nzakas

@nzakas nzakas commented Sep 18, 2026 •

Copy link
Copy Markdown
Member

Adds shell/require-read-r, which mirrors ShellCheck SC2162: read without -r treats backslashes as escapes and mangles input.

Behavior

  • Reports read when none of its short-flag arguments before -- contains r. Combined flags like -rs and separate flags like -t 5 -r both count.
  • Autofix: inserts -r right after read (read -s pw → read -r -s pw).
  • Recommended config: "error".

Documentation

Adds docs/rules/require-read-r.md, following the format of the @eslint/json, @eslint/css, and @eslint/markdown rule docs: description, background, rule details with incorrect and correct examples, options, when not to use it, and the ShellCheck reference. The rule's meta.docs.url points at that file, and its README table entry links to it.

Testing

  • 8 RuleTester cases.
  • An end-to-end verifyAndFix test in tests/autofix.test.ts.
  • 5 documentation checks.

284 tests total; build, lint, and format checks pass.

🤖 Generated with Claude Code


Stack created with GitHub Stacks CLI • Give Feedback 💬

@coderabbitai

coderabbitai Bot commented Sep 18, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 35 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 7d3d4563-9a34-4857-8ab6-1ff9d7728b99

📥 Commits

Reviewing files that changed from the base of the PR and between cade9ff and 99a4eeb.

📒 Files selected for processing (7)
  • README.md
  • docs/rules/require-read-r.md
  • src/index.spec.ts
  • src/index.ts
  • src/rules/require-read-r.spec.ts
  • src/rules/require-read-r.ts
  • tests/autofix.test.ts
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@nzakas
nzakas added this pull request to stack #16 September 18, 2026 15:00
@nzakas
nzakas marked this pull request as ready for review September 18, 2026 15:01
@nzakas
nzakas force-pushed the rule/require-read-r branch 2 times, most recently from 97e8f63 to 6759d09 Compare September 22, 2026 19:56
@nzakas
nzakas force-pushed the rule/require-read-r branch from 6759d09 to 91fdea3 Compare September 22, 2026 20:23
@nzakas
nzakas force-pushed the rule/require-read-r branch 2 times, most recently from 85ef581 to be6c318 Compare September 22, 2026 21:21
@nzakas
nzakas requested a balanced review from Copilot September 28, 2026 18:25

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

Option operands containing -r can suppress valid reports even though raw mode is not enabled.

Review effort: Balanced
Findings: 1 Medium severity

Open (1)
What changed in this PR

Adds the recommended bash/require-read-r rule to prevent read from mangling backslashes.

Changes:

  • Implements and registers the autofixable rule.
  • Adds unit, integration, and documentation coverage.
  • Documents the rule and updates the README.
File Description
src/​rules/​require-read-r.ts Implements detection and autofixing.
src/​rules/​require-read-r.spec.ts Adds rule tests.
src/​index.ts Registers and recommends the rule.
src/​index.spec.ts Verifies rule registration.
tests/​autofix.test.ts Adds end-to-end autofix coverage.
docs/​rules/​require-read-r.md Documents behavior and usage.
README.md Adds the rule to the catalog.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/rules/require-read-r.ts Outdated
Comment on lines +39 to +40
if (text !== null && /^-[a-zA-Z]*r/u.test(text)) {
return;

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Already addressed in 7b4a7d1: the rule now walks read's option clusters with arity, so the operand of -d (separate or attached) is not mistaken for -r. Both read -d -r value and read -dr value are covered as invalid cases. Reviewed and confirmed the suite passes on this branch.

@nzakas

nzakas commented Sep 29, 2026

Copy link
Copy Markdown
Member Author

@copilot address review comments. Be sure to add tests.

Copilot AI commented Sep 29, 2026

Copy link
Copy Markdown

@copilot address review comments. Be sure to add tests.

Fixed in commit 7b4a7d1; added regression tests for separate and attached delimiter operands.

nzakas and others added 2 commits October 2, 2026 12:18
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-authored-by: nzakas <38546+nzakas@users.noreply.github.com>
@nzakas
nzakas force-pushed the rule/require-read-r branch from d3ee679 to 99a4eeb Compare October 2, 2026 16:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants