Skip to content

fix: lint comments after shebangs#3199

Merged
elijah-potter merged 1 commit intoAutomattic:masterfrom
lawrence3699:fix/lint-comments-after-shebangs
Apr 20, 2026
Merged

fix: lint comments after shebangs#3199
elijah-potter merged 1 commit intoAutomattic:masterfrom
lawrence3699:fix/lint-comments-after-shebangs

Conversation

@lawrence3699
Copy link
Copy Markdown
Contributor

Disclaimer: This PR was authored by an agent and reviewed against the reproduced issue and local test results.

Fixes #962.

Before this change, CommentMasker ignored any merged comment span that started with a shebang. When tree-sitter merged the first-line shebang with the following comment block, the later comments were skipped too.

This change only strips the first line of a real shebang at the start of the file, then keeps linting the rest of the merged comment span.

How I tested this:

  • cargo test -p harper-comments
  • cargo run -q -p harper-cli -- lint /tmp/harper-962-with-shebang.sh --count --no-color
  • cargo run -q -p harper-cli -- lint /tmp/harper-962-no-shebang.sh --count --no-color
  • cargo run -q -p harper-cli -- lint /tmp/harper-962-fake-shebang.sh --count --no-color

Copilot AI review requested due to automatic review settings April 17, 2026 22:16
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes #962 by ensuring CommentMasker no longer drops an entire merged comment span when it begins with a shebang; instead it strips only the first-line shebang at the start of the file and continues linting subsequent comments.

Changes:

  • Update CommentMasker::create_mask to trim a leading first-line shebang (only when span.start == 0) rather than ignoring the entire merged span.
  • Add a regression fixture (issue_962.sh) and corresponding test expectation to ensure comments after a shebang are linted.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
harper-comments/src/masker.rs Adjusts masking logic to trim only the initial shebang line (if present at file start) while preserving linting of subsequent merged comment content.
harper-comments/tests/language_support_sources/issue_962.sh Adds a shell script fixture reproducing the shebang+comment scenario from #962.
harper-comments/tests/language_support.rs Registers the new fixture in the language support test suite with the expected lint count.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Copy Markdown
Collaborator

@elijah-potter elijah-potter left a comment

Choose a reason for hiding this comment

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

This looks like a simple enough change to me. Thanks!

@elijah-potter elijah-potter added this pull request to the merge queue Apr 20, 2026
Merged via the queue into Automattic:master with commit c0be66f Apr 20, 2026
14 of 15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

harper-ls ignores comments between shebang and code

3 participants