Skip to content

feat: add Swift/PHP/Kotlin/Shell post-edit diagnostics - #44

Merged
aakashrajput merged 2 commits into
KlaatAI:mainfrom
Ayush7614:feat/more-language-diagnostics
Jul 21, 2026
Merged

feat: add Swift/PHP/Kotlin/Shell post-edit diagnostics#44
aakashrajput merged 2 commits into
KlaatAI:mainfrom
Ayush7614:feat/more-language-diagnostics

Conversation

@Ayush7614

@Ayush7614 Ayush7614 commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Extends post-edit diagnostics with PATH-gated linters for Swift (swiftlint), PHP (phpstanpintphp -l), Kotlin (ktlint), and Shell (shellcheck).
  • Tools are never auto-installed; missing binaries are silently skipped (same pattern as Ruby/rubocop).
  • Adds unit coverage and updates the README diagnostics list.

Closes #2

Test plan

  • bun run typecheck
  • bun test (190 pass)
  • bun run bench:selfcheck
  • bun run build
  • Spot-check with a tool on PATH (e.g. shellcheck) editing a .sh file in the TUI

Extend PATH-gated diagnostics with swiftlint, phpstan/pint/php -l,
ktlint, and shellcheck so more languages get in-loop lint feedback
after edits without auto-installing tools.
@github-actions

Copy link
Copy Markdown
Contributor

🤖 KlaatAI Review Bot (powered by Klaatu, advisory only — a maintainer makes the real call)

Issue match
No linked issue (null). Reviewers must manually confirm the PR's scope matches the intended feature request.

Test coverage
Tests were added in src/tools/diagnostics.test.ts. They cover negative paths: ensuring no exceptions when binaries are absent, returning null when disabled, and verifying explicit config overrides win. Missing is positive-path coverage (e.g., mocking onPath to return true) to verify the correct CLI arguments are actually constructed for the spawned processes.

Correctness concerns
In src/tools/diagnostics.ts, the PHP fallback chain (phpstanpintphp -l) mixes semantic levels. phpstan and pint check style/static analysis, while php -l only checks syntax. If a user has php but not the others, they silently get a much weaker diagnostic. Additionally, verify ktlint --reporter=plain compatibility, as ktlint 1.0.0+ changed reporter flags.

Verdict
Needs human judgment call on whether the PHP php -l fallback is acceptable and if ktlint CLI flags need version-gating.

This is an automated review to help triage faster, not a gate. Nothing here blocks merging.

@aakashrajput

Copy link
Copy Markdown
Member

@Ayush7614 please link the issue to your PR, Thanks

@Ayush7614

Copy link
Copy Markdown
Contributor Author

Linked to #2 (shellcheck / shell script diagnostics). This PR also adds the same PATH-gated pattern for Swift (swiftlint), PHP (phpstan/pint/php -l), and Kotlin (ktlint) alongside shellcheck.

@github-actions

Copy link
Copy Markdown
Contributor

🤖 KlaatAI Review Bot (powered by Klaatu, advisory only — a maintainer makes the real call)

Issue match
The PR fully implements the shell script diagnostics requested in #2, but significantly expands scope by also adding Swift, PHP, and Kotlin. Issue #2 was strictly scoped to shell scripts.

Test coverage
Tests in src/tools/diagnostics.test.ts cover the "tool absent" and "diagnostics disabled" paths for all new extensions, plus config overrides. However, they lack coverage for the "tool present" execution path. A test mocking onPath to return true is needed to verify actual command construction and output parsing for these new linters.

Correctness concerns
In src/tools/diagnostics.ts, the shell branch includes .zsh. shellcheck explicitly does not support zsh and will emit false-positive errors for valid zsh syntax; .zsh should be removed from this block. Additionally, the PHP fallback to pint may yield confusing results on non-Laravel projects if the binary is globally installed.

Verdict
Needs human judgment call on scope creep (PR adds three languages beyond Issue #2's shell-only request) and a specific fix to remove .zsh from the shellcheck branch.

This is an automated review to help triage faster, not a gate. Nothing here blocks merging.

Drop .zsh from shellcheck (unsupported), remove php -l fallback,
simplify ktlint argv for 1.x compatibility, and add positive-path
tests via injectable PATH stubs on resolveDiagnosticsCommand.
@Ayush7614

Copy link
Copy Markdown
Contributor Author

Addressed the review-bot notes in a follow-up commit:

  • Removed .zsh from the shellcheck branch (shellcheck doesn’t support zsh)
  • Dropped the php -l fallback (kept phpstanpint only)
  • Simplified ktlint argv (no --reporter flag for 1.x compatibility)
  • Added positive-path tests via injectable PATH stubs on resolveDiagnosticsCommand

@github-actions

Copy link
Copy Markdown
Contributor

🤖 KlaatAI Review Bot (powered by Klaatu, advisory only — a maintainer makes the real call)

Issue match
Partial. The PR fully implements the Shell diagnostics requested in issue #2, but significantly scope-creeps by also adding Swift, PHP, and Kotlin support, which were not requested in the linked issue.

Test coverage
Strong coverage in src/tools/diagnostics.test.ts. Tests verify soft-skips when tools are absent, disabled states, exact argv shapes via injected onPath stubs, config overrides, and explicitly assert .zsh is ignored. Missing: the PR's manual test plan checkbox for spot-checking a real tool on PATH remains unchecked, though CI coverage of the resolution logic is thorough.

Correctness concerns
Minor discrepancy: the PR body claims a php -l fallback for PHP, but src/tools/diagnostics.ts explicitly omits it. The code's choice is likely correct, but the PR description should be updated to match. Additionally, verify that ktlint's default plain output (without --reporter) doesn't cause parsing issues in the downstream runDiagnostics executor, given ktlint 1.x CLI changes.

Verdict
Needs human judgment call on whether to accept the scope creep (Swift/PHP/Kotlin) beyond issue #2's Shell-only request, and to confirm ktlint output parsing compatibility.

This is an automated review to help triage faster, not a gate. Nothing here blocks merging.

@aakashrajput
aakashrajput merged commit 4cc0e71 into KlaatAI:main Jul 21, 2026
2 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.

Add shell script diagnostics support (shellcheck)

2 participants