Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Missing information about applied rector #5139

Closed
Wirone opened this issue Jan 11, 2021 · 7 comments
Closed

Missing information about applied rector #5139

Wirone opened this issue Jan 11, 2021 · 7 comments
Labels

Comments

@Wirone
Copy link
Contributor

Wirone commented Jan 11, 2021

Bug Report

Subject Details
Rector version e.g. v0.8.56
Installed as composer dependency

I have weird issue with Rector, that tries to change my Doctrine annotation but does not print information about applied rectors so I can't fix my configuration in order to get rid of these unwanted changes:

--- Original
+++ New
@@ -69,7 +69,7 @@
     protected ?bool $ebook = false;

     /**
-     * @ORM\Column(type="integer", options={"default": 0})
+     * @ORM\Column(type="integer", options={"default"=0})
      */
     protected int $attempt = 0;

@@ -95,7 +95,7 @@
      *     "diet_list"
      * })
      *
-     * @ORM\Column(type="integer", options={"default": 1})
+     * @ORM\Column(type="integer", options={"default"=1})
      */
     protected int $periodInMonths = 1;
--- Original
+++ New
@@ -13,11 +13,9 @@
  *     indexes={
  *         @ORM\Index(name="idx_content", columns={"content"}, flags={"fulltext"})
  *     },
- *     uniqueConstraints={
- *         @ORM\UniqueConstraint(name="lookup_unique_idx", columns={
- *             "locale", "object_id", "field"
- *         })}
- *     )
+ *     uniqueConstraints={@ORM\UniqueConstraint(name="lookup_unique_idx",
+ *         columns={locale, object_id,
+ *             field}) } )
  */
 class Foo extends Bar
 {

For the other issues I have something like this:

Applied rules:

 * Rector\CodeQuality\Rector\Foreach_\ForeachItemsAssignToEmptyArrayToAssignRector
 * Rector\CodeQuality\Rector\If_\ExplicitBoolCompareRector

but not for changes from diffs above. For each file with annotation-related changes I see only file name and diff.

Expected Behaviour

First of all, there should be info about applied rectors. Without that it is hard to alter configuration.

Second, I don't want Rector to change annotations convention (:=) since I have ECS configured for that (DoctrineAnnotationArrayAssignmentFixer + DoctrineAnnotationSpacesFixer). Third: stripping quotation marks causes AnnotationException because library treats this value as constant name and it's not found. Last but not least, I would expect that Rector does not make formatting worse than it was (or at least that ECS can fix it after Rector, which isn't the case here (running Rector + ECS leaves ugly-formatted annotations).

@Wirone Wirone added the bug label Jan 11, 2021
@Wirone
Copy link
Contributor Author

Wirone commented Jan 11, 2021

FYI: I can't use Rector 0.9 because I have Symfony 4.4 and there is constraint for symfony/dependency-injection ^5.1 in symplify/autowire-array-parameter (which is weird because other packages have ^4.4 | ^5.1).

@TomasVotruba
Copy link
Member

Hi,

Rector 0.8 is not maintained, only higher version are.
For version conflicts, there is prefixed version: https://github.com/rectorphp/rector-prefixed

This issue is sub-issue of #4334
Untils that one is resolved, these docblocks typos will happen

@Wirone
Copy link
Contributor Author

Wirone commented Jan 11, 2021

Thanks @TomasVotruba for the info. Actually I was able to upgrade project to Symfony 5.2 so I could update Rector and ECS to 0.9 and 9.0 respectively. Seems to be working 🙂

@TomasVotruba
Copy link
Member

That's good news :) congrats!

How is Symfony 5.2 and PHP 8?
I've found few new combos:
https://tomasvotruba.com/blog/2020/12/21/5-new-combos-opened-by-symfony-52-and-php-80/

@Wirone
Copy link
Contributor Author

Wirone commented Jan 14, 2021

@TomasVotruba I've been working more in devops/QA area last few days so I can't say much about Symfony 5.2 and PHP 8 😉 I'll work with code in next days, so I'll see, but looks optimistic.

One thing I've noticed Rector did not solve after upgrading Symfony from 4.4 to 5.2 was JsonResponse::create() usage (deprecated in Symfony 5.1). Rector for this would be great.

PS. I've tried to catch you on Symfony Slack because I have really weird problem with ECS that I struggle to debug. Did not create issue because it's only CI issue, locally works fine. What is the best way to get some help on it?

@TomasVotruba
Copy link
Member

TomasVotruba commented Jan 14, 2021

One thing I've noticed Rector did not solve after upgrading Symfony from 4.4 to 5.2 was JsonResponse::create() usage (deprecated in Symfony 5.1). Rector for this would be great.

If you make a diff for this in #4824, we can give it priority 👍

What is the best way to get some help on it?

I don't use any Slack chanells, as it's a big productivity killer :). The best way is reproducible issue on GitHub, e.g. failing GitHub Action in your repository. Those get resolved by our team with priority

TomasVotruba added a commit that referenced this issue Oct 7, 2023
@rela589n
Copy link

rela589n commented Jul 3, 2024

It seems that there's an regression on it:

1) NotificationCampaign.php:37

    ---------- begin diff ----------
@@ @@
     private NotificationCampaignSchedule $schedule;

     #[ORM\Column]
-    private NotificationCampaignStatus $status;
+    private NotificationCampaignStatus $status = NotificationCampaignStatus::DRAFT;

     #[ORM\Column(type: 'carbon_immutable')]
     private CarbonImmutable $createdAt;
@@ @@
         $this->details = $command->getNotificationDetails();
         $this->schedule = $command->getSchedule();
-        $this->status = NotificationCampaignStatus::DRAFT;
         $this->createdAt = CarbonImmutable::now();
    ----------- end diff -----------

                                                                                                                        
 [OK] 1 file would have changed (dry-run) by Rector                                                                                                                                                                                         

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants