Add Mago as task

[johnatas-x]

Q	A
Branch	mago-task
Bug fix?	no
New feature?	yes
BC breaks?	no
Deprecations?	no
Documented?	yes
Fixed tickets	#1193

This PR adds a Mago task

Mago is a recent tool that includes a formatter (like phpcs), a linter (like phpmd), and a static analyzer (like phpstan).
It is known for its performance and is starting to be increasingly adopted (for example, in Drupal).
Its integration into GrumPHP seems valuable.

New Task Checklist:

• Are the dependencies added to the composer.json suggestions?
• Is the doc/tasks.md file updated?
• Are the task parameters documented?
• Is the task registered in the tasks.yml file?
• Does the task contains phpunit tests?
• Is the configuration having logical allowed types?
• Does the task run in the correct context?
• Is the run() method readable?
• Is the run() method using the configuration correctly?
• Are all CI services returning green?

[veewee]

Thanks for the PR.
I think this task in current state is not in line with other tasks. I've added some remarks inline in the code. Feel free to stat a discussion.

[johnatas-x]

Hey @veewee,

Thanks for taking the time to review the PR and share your feedback.
Your comments align with the questions I raised in the related issue.

To be fully transparent, after opening the PR I had already started reworking it to introduce 4 commands. I’ll update the PR this week with those changes; it should end up closer to how the SecurityChecker tasks are structured, which should resolve the three threads.

If you have any additional feedback, feel free to share it so I can address everything at once.

[johnatas-x]

Hey,

I just pushed a refactor splitting the mago task into 4 distinct subtasks: mago_lint, mago_analyze, mago_format, and mago_guard. Each task now follows the same structure as other tasks, with its own configurable options, dedicated documentation, and unit tests.

The mago task itself now serves two purposes: it acts as a redirect, pointing users to the 4 subtasks when invoked directly, and as a base class holding the shared logic to avoid code duplication across the subtasks.

Happy to hear any feedback — whether on the option coverage, naming, documentation, or anything else that could be improved.

[veewee]

Thanks for the changes.
I currently only looked at the tasks and added some comments.
In general, I think we should do things a bit different here and there.

[johnatas-x]

No worries
Did you add comments? I can’t see any

[johnatas-x]

Hi @veewee,

Thanks for all your feedback!

I'm working through it bit by bit. I'll push a new version this weekend that incorporates your comments, and I'll also include answers to your questions.

Talk to you soon.

[johnatas-x]

Hi, here is a summary of all the changes made based on your feedback.

---

staged option removed from config — auto-detected from context

The staged flag is no longer a user-facing option. It is now automatically passed to mago lint / mago analyze when running in a GitPreCommitContext, and omitted in RunContext. This matches GrumPHP's philosophy of letting the context drive behavior rather than the user config.

On that note: I kept --staged (mago's native flag) rather than passing file paths from the diff context. The reason is that files and directories processed by Mago are defined in mago.toml. Passing explicit paths from GrumPHP's diff would override that configuration — meaning a file not covered by mago.toml could end up being linted/analyzed/formatted unintentionally. Using --staged delegates the file selection entirely to Mago's own configuration, which is the safe and expected behavior.

---

mago lint / mago guard — always run in dry-run + fix mode

These two tasks always run with --fix --dry-run --fail-on-remaining. When the task fails, GrumPHP offers to re-run with --fix applied (without --dry-run). The fix aggressiveness is controlled by a new fix-mode option (safe by default, potentially-unsafe, unsafe).

---

mago analyze — runs directly for full diagnostic output

The analyzer runs mago analyze without --fix --dry-run, as combining those flags suppresses the detailed diagnostic output. When the task fails, GrumPHP offers a fix command (mago analyze --fix) built separately, with the appropriate fix-mode flag if configured.

---

Removed options incompatible with --fix

fixable-only, reporting-format, reporting-target, and minimum-fail-level have been removed from the shared options as they cannot be used alongside --fix. fail-on-remaining is now hardcoded (always on) since it is required to detect unfixed issues during the dry-run step.

---

mago_formatter — always runs --dry-run, offers fix in both contexts

The type option has been removed. The formatter always runs with --dry-run. In both GitPreCommitContext and RunContext, GrumPHP offers to re-run without --dry-run to apply formatting in-place.

---

mago_guard — checks replaced by structural and perimeter bools

Following your feedback, the custom checks string option has been replaced by two independent boolean flags structural and perimeter, consistent with how other options are handled across Mago tasks.

---

Abstract Mago class kept

I kept the Mago abstract class, which is extended by MagoAnalyzer, MagoLinter, and MagoGuard. These three share the same execution pattern (shared options, --fix --dry-run, createFailedWithFix). MagoFormatter now extends AbstractExternalTask directly as it has its own distinct behavior and does not benefit from the shared abstract.

Are you happy with this approach, or would you prefer I remove the abstract and duplicate its content across the three tasks?

---

I ran a few tests on a real project and things seem to work as expected. That said, my Mago configuration is fairly minimal, so edge cases with more advanced setups (custom rules, baseline files, analyzer stubs, etc.) haven't been covered. Additional testing from someone with a more complete Mago configuration would be welcome.

Happy to address any further feedback, thanks!

[veewee]

Hey @johnatas-x,

Rather than going back and forth on the open threads another dozen times, I pushed a version of how I think the Mago tasks could look. Easier to react to something concrete than to keep describing it.

The biggest change: I dropped the abstract Mago class. It was trying to share logic across all four commands, but the commands aren't really that alike. Guard can't auto-fix, format handles staged files differently, and the linter/analyzer don't even run the same way. So the base class ended up carrying behaviour that only fit some of them, which made the whole thing harder to follow than it should be. Each command is now a standalone task you can read top to bottom.

While I was in there I lined the tasks up with how the rest of GrumPHP behaves (dry-run by default, offer to fix when it makes sense) and with what Mago actually supports. I went through every flag against the real mago CLI, so guard no longer pretends it can fix things, and the structural/perimeter choice is now a single mode option since you can't use both at once anyway.

Tests cover each option per task and the fix commands they generate, and the docs are updated to match.

Could you pull it and try it on a real project, especially the pre-commit runs? Want to make sure it behaves the way you'd expect on a setup more complete than mine. If it feels right to you, I'll get it merged.

[johnatas-x]

Hi @veewee,

Thank you for the effort you've put into this.

I'm going to review your changes to better understand the approach and improve my own work going forward.

I'll test the branch on three real projects during the course of next week and will get back to you with feedback based on those tests.