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

[veewee]

Maybe better to have it abstract as wel? There is no such thing as a mago task

[veewee]

not needed when abstract.

[veewee]

not needed when abstract.

[veewee]

lets not add the fix option.
Rather use something like this:

grumphp/src/Task/PhpCsFixer.php

Lines 107 to 116 in 732a228

	if (!$process->isSuccessful()) {
	return FixableProcessResultProvider::provide(
	TaskResult::createFailed($this, $context, $this->formatter->format($process)),
	function () use ($arguments): Process {
	$arguments->removeElement('--format=json');
	$arguments->removeElement('--dry-run');
	return $this->processBuilder->buildProcess($arguments);
	}
	);
	}

By default grumphp does dry-run.
When failed, it asks if you want it to be fixed or not (meaning it 'll run the fix command)

[veewee]

remove dry-run option.

[veewee]

does it make sense to have staged here?
We could possibly automatically do this during pre-commit and dont do it during run context?

That way we don't have to pass files ever.. Otherwise we'dd need to pass files during pre-commit.

[veewee]

lets not hide options behind a custom 'type'

[veewee]

lets not hide options behind a custom 'checks'. make them bools.

[veewee]

see previous feedback: lets make staged or not dependant of the context.

[veewee]

are those options actually shared across all the commands in mago as well?
Because this could bite us if not. For example: see formatter tasks, which is not using the shared options.