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

PHP attributes usage cause unexpected changes #9334

Closed
mpiot opened this issue Jan 7, 2022 · 4 comments
Closed

PHP attributes usage cause unexpected changes #9334

mpiot opened this issue Jan 7, 2022 · 4 comments

Comments

@mpiot
Copy link

mpiot commented Jan 7, 2022

Bug Report

Q A
BC Break
Version 2.10.4

Summary

In a project I try to update from Doctrine Annotations to PHP Attributes, but it causing unexpected schema changes. (Replace the doctrine/migrations#1222)

Current behavior

Previous annotations:

/**
 * @ORM\ManyToMany(targetEntity=User::class)
 * @ORM\JoinColumn(nullable=false)
 */
private Collection $inventors;

New attributes:

#[ORM\ManyToMany(targetEntity: User::class)]
#[ORM\JoinColumn(nullable: false)]
private Collection $inventors;

When executing a migrations we have something like:

public function up(Schema $schema): void
 {
    // this up() migration is auto-generated, please modify it to your needs
    $this->addSql('ALTER TABLE soleau_envelope_user DROP CONSTRAINT FK_A06D2FACA2F59D5');
    $this->addSql('ALTER TABLE soleau_envelope_user ADD CONSTRAINT FK_A06D2FACA2F59D5 FOREIGN KEY (soleau_envelope_id) REFERENCES soleau_envelope (id) NOT DEFERRABLE INITIALLY IMMEDIATE');
}

public function down(Schema $schema): void
{
    // this down() migration is auto-generated, please modify it to your needs
    $this->addSql('ALTER TABLE soleau_envelope_user DROP CONSTRAINT fk_a06d2faca2f59d5');
    $this->addSql('ALTER TABLE soleau_envelope_user ADD CONSTRAINT fk_a06d2faca2f59d5 FOREIGN KEY (soleau_envelope_id) REFERENCES soleau_envelope (id) ON DELETE CASCADE NOT DEFERRABLE INITIALLY IMMEDIATE');
}

If I dump fromSchema and toSchema from the Doctrine/Migration DiffGenerator, I have something like:

Before:

Doctrine\DBAL\Schema\Table^ {#90
  #_name: "soleau_envelope_user"
  #_namespace: null
  #_quoted: false
  #_columns: array:2 [
    // ...
  ]
  #_indexes: array:3 [
    "idx_a06d2faca2f59d5" => Doctrine\DBAL\Schema\Index^ {#106
      #_name: "idx_a06d2faca2f59d5"
      #_namespace: null
      #_quoted: false
      #_columns: array:1 [
        "soleau_envelope_id" => Doctrine\DBAL\Schema\Identifier^ {#107
          #_name: "soleau_envelope_id"
          #_namespace: null
          #_quoted: false
        }
      ]
      #_isUnique: false
      #_isPrimary: false
      #_flags: []
      -options: array:1 [
        "lengths" => array:1 [
          0 => null
        ]
      ]
    }
    "idx_a06d2faa76ed395" => Doctrine\DBAL\Schema\Index^ {#108
      #_name: "idx_a06d2faa76ed395"
      #_namespace: null
      #_quoted: false
      #_columns: array:1 [
        "user_id" => Doctrine\DBAL\Schema\Identifier^ {#73
          #_name: "user_id"
          #_namespace: null
          #_quoted: false
        }
      ]
      #_isUnique: false
      #_isPrimary: false
      #_flags: []
      -options: array:1 [
        "lengths" => array:1 [
          0 => null
        ]
      ]
    }
    "soleau_envelope_user_pkey" => Doctrine\DBAL\Schema\Index^ {#91
      #_name: "soleau_envelope_user_pkey"
      #_namespace: null
      #_quoted: false
      #_columns: array:2 [
        "soleau_envelope_id" => Doctrine\DBAL\Schema\Identifier^ {#93
          #_name: "soleau_envelope_id"
          #_namespace: null
          #_quoted: false
        }
        "user_id" => Doctrine\DBAL\Schema\Identifier^ {#62
          #_name: "user_id"
          #_namespace: null
          #_quoted: false
        }
      ]
      #_isUnique: true
      #_isPrimary: true
      #_flags: []
      -options: array:1 [
        "lengths" => array:2 [
          0 => null
          1 => null
        ]
      ]
    }
  ]
  #_primaryKeyName: "soleau_envelope_user_pkey"
  #uniqueConstraints: []
  #_fkConstraints: array:2 [
    "fk_a06d2faa76ed395" => Doctrine\DBAL\Schema\ForeignKeyConstraint^ {#123
      #_name: "fk_a06d2faa76ed395"
      #_namespace: null
      #_quoted: false
      #_localTable: Doctrine\DBAL\Schema\Table^ {#90}
      #_localColumnNames: array:1 [
        "user_id" => Doctrine\DBAL\Schema\Identifier^ {#117
          #_name: "user_id"
          #_namespace: null
          #_quoted: false
        }
      ]
      #_foreignTableName: Doctrine\DBAL\Schema\Identifier^ {#116
        #_name: "app_user"
        #_namespace: null
        #_quoted: false
      }
      #_foreignColumnNames: array:1 [
        "id" => Doctrine\DBAL\Schema\Identifier^ {#113
          #_name: "id"
          #_namespace: null
          #_quoted: false
        }
      ]
      #_options: array:2 [
        "onUpdate" => null
        "onDelete" => "CASCADE"
      ]
    }
    "fk_a06d2faca2f59d5" => Doctrine\DBAL\Schema\ForeignKeyConstraint^ {#112
      #_name: "fk_a06d2faca2f59d5"
      #_namespace: null
      #_quoted: false
      #_localTable: Doctrine\DBAL\Schema\Table^ {#90}
      #_localColumnNames: array:1 [
        "soleau_envelope_id" => Doctrine\DBAL\Schema\Identifier^ {#111
          #_name: "soleau_envelope_id"
          #_namespace: null
          #_quoted: false
        }
      ]
      #_foreignTableName: Doctrine\DBAL\Schema\Identifier^ {#110
        #_name: "soleau_envelope"
        #_namespace: null
        #_quoted: false
      }
      #_foreignColumnNames: array:1 [
        "id" => Doctrine\DBAL\Schema\Identifier^ {#109
          #_name: "id"
          #_namespace: null
          #_quoted: false
        }
      ]
      #_options: array:2 [
        "onUpdate" => null
        "onDelete" => "CASCADE"
      ]
    }
  ]
  #_options: array:2 [
    "create_options" => []
    "comment" => null
  ]
  #_schemaConfig: Doctrine\DBAL\Schema\SchemaConfig^ {#10711
    #hasExplicitForeignKeyIndexes: false
    #maxIdentifierLength: 63
    #name: "symfony"
    #defaultTableOptions: array:1 [
      "charset" => "utf8"
    ]
  }
  -implicitIndexes: []
}

After (with attributes):

^ Doctrine\DBAL\Schema\Table^ {#20659
  #_name: "soleau_envelope_user"
  #_namespace: null
  #_quoted: false
  #_columns: array:2 [
    // ...
  ]
  #_indexes: array:3 [
    "idx_a06d2faca2f59d5" => Doctrine\DBAL\Schema\Index^ {#20665
      #_name: "IDX_A06D2FACA2F59D5"
      #_namespace: null
      #_quoted: false
      #_columns: array:1 [
        "soleau_envelope_id" => Doctrine\DBAL\Schema\Identifier^ {#20666
          #_name: "soleau_envelope_id"
          #_namespace: null
          #_quoted: false
        }
      ]
      #_isUnique: false
      #_isPrimary: false
      #_flags: []
      -options: []
    }
    "idx_a06d2faa76ed395" => Doctrine\DBAL\Schema\Index^ {#20672
      #_name: "IDX_A06D2FAA76ED395"
      #_namespace: null
      #_quoted: false
      #_columns: array:1 [
        "user_id" => Doctrine\DBAL\Schema\Identifier^ {#20673
          #_name: "user_id"
          #_namespace: null
          #_quoted: false
        }
      ]
      #_isUnique: false
      #_isPrimary: false
      #_flags: []
      -options: []
    }
    "primary" => Doctrine\DBAL\Schema\Index^ {#20674
      #_name: "primary"
      #_namespace: null
      #_quoted: false
      #_columns: array:2 [
        "soleau_envelope_id" => Doctrine\DBAL\Schema\Identifier^ {#20675
          #_name: "soleau_envelope_id"
          #_namespace: null
          #_quoted: false
        }
        "user_id" => Doctrine\DBAL\Schema\Identifier^ {#20676
          #_name: "user_id"
          #_namespace: null
          #_quoted: false
        }
      ]
      #_isUnique: true
      #_isPrimary: true
      #_flags: []
      -options: []
    }
  ]
  #_primaryKeyName: "primary"
  #uniqueConstraints: []
  #_fkConstraints: array:2 [
    "fk_a06d2faca2f59d5" => Doctrine\DBAL\Schema\ForeignKeyConstraint^ {#20661
      #_name: "FK_A06D2FACA2F59D5"
      #_namespace: null
      #_quoted: false
      #_localTable: Doctrine\DBAL\Schema\Table^ {#20659}
      #_localColumnNames: array:1 [
        "soleau_envelope_id" => Doctrine\DBAL\Schema\Identifier^ {#20662
          #_name: "soleau_envelope_id"
          #_namespace: null
          #_quoted: false
        }
      ]
      #_foreignTableName: Doctrine\DBAL\Schema\Identifier^ {#20663
        #_name: "soleau_envelope"
        #_namespace: null
        #_quoted: false
      }
      #_foreignColumnNames: array:1 [
        "id" => Doctrine\DBAL\Schema\Identifier^ {#20664
          #_name: "id"
          #_namespace: null
          #_quoted: false
        }
      ]
      #_options: []
    }
    "fk_a06d2faa76ed395" => Doctrine\DBAL\Schema\ForeignKeyConstraint^ {#20668
      #_name: "FK_A06D2FAA76ED395"
      #_namespace: null
      #_quoted: false
      #_localTable: Doctrine\DBAL\Schema\Table^ {#20659}
      #_localColumnNames: array:1 [
        "user_id" => Doctrine\DBAL\Schema\Identifier^ {#20669
          #_name: "user_id"
          #_namespace: null
          #_quoted: false
        }
      ]
      #_foreignTableName: Doctrine\DBAL\Schema\Identifier^ {#20670
        #_name: "app_user"
        #_namespace: null
        #_quoted: false
      }
      #_foreignColumnNames: array:1 [
        "id" => Doctrine\DBAL\Schema\Identifier^ {#20671
          #_name: "id"
          #_namespace: null
          #_quoted: false
        }
      ]
      #_options: array:1 [
        "onDelete" => "CASCADE"
      ]
    }
  ]
  #_options: array:2 [
    "create_options" => []
    "charset" => "utf8"
  ]
  #_schemaConfig: Doctrine\DBAL\Schema\SchemaConfig^ {#14544
    #hasExplicitForeignKeyIndexes: false
    #maxIdentifierLength: 63
    #name: "symfony"
    #defaultTableOptions: array:1 [
      "charset" => "utf8"
    ]
  }
  -implicitIndexes: array:2 [
    "idx_a06d2faca2f59d5" => Doctrine\DBAL\Schema\Index^ {#20665}
    "idx_a06d2faa76ed395" => Doctrine\DBAL\Schema\Index^ {#20672}
  ]
}

Expected behavior

By changing from DoctrineAnnotations to PHP Attributes, this change should not appear. This appear 10 times for 51 ManyToMany relations.

The change is always the same, we loose the 'ON DELETE CASCADE' and the constraint name is uppercase.

@derrabus
Copy link
Member

derrabus commented Jan 7, 2022

On a ManyToMany relation, JoinColumn annotations have to be wrapped into a JoinTable annotation to have an effect. With attributes however, that nesting is not necessary anymore.

Just delete JoinColumn, it was not taken into account previously, so you probably don't need it.

@mpiot
Copy link
Author

mpiot commented Jan 8, 2022

Thanks a lot @derrabus you're totally right. Sorry for this issue that is not an issue.

@mpiot mpiot closed this as completed Jan 8, 2022
@ndench
Copy link

ndench commented Jan 27, 2022

As an FYI for anyone else who stumbles across here from Google. You also get unexpected changes if you have @JoinTable annotations with nested @JoinColumn annotations such as:

/**
 * @ORM\ManyToMany(targetEntity="App\Entity\Foo")
 * @ORM\JoinTable(
 *     name="bar_foo",
 *     joinColumns={@ORM\JoinColumn(name="bar_id", referencedColumnName="id")},
 *     inverseJoinColumns={@ORM\JoinColumn(name="foo_id", referencedColumnName="id", unique=true)}
 * )
 */

There's a slight note in the Doctrine Attributes Reference:

A notable difference to the annotation metadata support, #[JoinColumn] and #[InverseJoinColumn] are specified at the property level and are not nested within the #[JoinTable] attribute.

Hence, to convert this to attributes you need:

#[ORM\ManyToMany(targetEntity: Foo::class)]
#[ORM\JoinTable(name: 'bar_foo')]
#[ORM\JoinColumn(name: 'bar_id', referencedColumnName: 'id')]
#[ORM\InverseJoinColumn(name: 'foo_id', referencedColumnName: 'id', unique: true)]

In fact, this is the case for all nested annotations. Another example is a UniqueConstraint:

/**
 * @ORM\Table(uniqueConstraints={@ORM\UniqueConstraint(columns={"email"})})
 * @ORM\Entity()
 */

Becomes:

#[ORM\Table]
#[ORM\UniqueConstraint(columns: ['email'])]
#[ORM\Entity]

@TomasVotruba
Copy link
Contributor

TomasVotruba commented Aug 18, 2022

Good news everyone 🙂
After few failed attempts, we've added a rule to handle these annotation unwrap to attributes. in rectorphp/rector-src#2781 - NestedAnnotationToAttributeRector

Here is clearly described what cases it covers: rectorphp/rector-doctrine#116 (comment),
apart 2 above, also Table->indexes unwrap to #[Index]

It is now in dev-main and will be part of Rector 0.14.1+

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

No branches or pull requests

4 participants