symfony / maker-bundle

Symfony Maker Bundle
https://symfony.com/
MIT License
3.34k stars 406 forks source link

[private $__EXTRA__LINE;]? Add field to related entity produces invalid code. #170

Closed ChanOmegaWebDesign closed 6 years ago

ChanOmegaWebDesign commented 6 years ago

Made an entity with ManyToOne, and the generated code on the One-side looks like this:

...
private $__EXTRA__LINE;
  /**
   * @ORM\OneToMany(targetEntity="App\Entity\BusinessDay", mappedBy="shop", orphanRemoval=true)
   */
  private $businessDays;
...

private $__EXTRA__LINE;
  /**
   * @return Collection|BusinessDay[]
   */
  public function getBusinessDays(): Collection
  {
      return $this->businessDays;
  }
  private $__EXTRA__LINE;
  public function addBusinessDay(BusinessDay $businessDay): self
  {
      if (!$this->businessDays->contains($businessDay)) {
          $this->businessDays[] = $businessDay;
          $businessDay->setShop($this);
      }
      $__EXTRA__LINE;
      return $this;
  }
  private $__EXTRA__LINE;
  public function removeBusinessDay(BusinessDay $businessDay): self
  {
      if ($this->businessDays->contains($businessDay)) {
          $this->businessDays->removeElement($businessDay);
          // set the owning side to null (unless already changed)
          if ($businessDay->getShop() === $this) {
              $businessDay->setShop(null);
          }
      }
      $__EXTRA__LINE;
      return $this;
  }

Here's what I did:

$ php bin/console make:entity

 Class name of the entity to create or update (e.g. BravePuppy):
 > BusinessDay
BusinessDay

 created: src/Entity/BusinessDay.php
 created: src/Repository/BusinessDayRepository.php

 Entity generated! Now let's add some fields!
 You can always add more fields later manually or by re-running this command.

 New property name (press <return> to stop adding fields):
 > shop

 Field type (enter ? to see all types) [string]:
 > ManyToOne
ManyToOne

 What class should this entity be related to?:
 > Shop
Shop

 Is the BusinessDay.shop property allowed to be null (nullable)? (
yes/no) [yes]:
 > no

 Do you want to add a new property to Shop so that you can access/update Bus
inessDay objects from it - e.g. $shop->getBusinessDays()? (yes/no) [yes
]:
 >

 A new property will also be added to the Shop class so that you can access the related Business
...
 New field name inside Shop [businessDays]:
 >

 Do you want to activate orphanRemoval on your relationship?
 A BusinessDay is "orphaned" when it is removed from its related Shop.
 e.g. $shop->removeBusinessDay($businessDay)

 NOTE: If a BusinessDay may *change* from one Shop to another, answer "no".

 Do you want to automatically delete orphaned App\Entity\BusinessDay objects (orphanRe
moval)? (yes/no) [no]:
 > yes

 updated: src/Entity/BusinessDay.php
 updated: src/Entity/Shop.php
stof commented 6 years ago

Can you give us the version of the bundle you are using (use composer show symfony/maker-bundle to get the info)

ChanOmegaWebDesign commented 6 years ago

Just updated (v1.4.3 => v1.4.4), then created a similar entity, and it appears to be fixed.

weaverryan commented 6 years ago

That is SUPER weird, and I don't know of why that would have been fixed in 1.4.4.

If you get this error again and can repeat it, let us know!

Thanks!

ChanOmegaWebDesign commented 6 years ago

It Happened again. Console:

$ php bin/console make:entity

 Class name of the entity to create or update (e.g. GentleChef):
 > CustomBootstrapTheme
CustomBootstrapThem[
Ke

 created: src/Entity/CustomBootstrapTheme.php
 created: src/Repository/CustomBootstrapThemeRepository.php

 Entity generated! Now let's add some fields!
 You can always add more fields later manually or by re-running this command.

 New property name (press <return> to stop adding fields):
 > shop

 Field type (enter ? to see all types) [string]:
 > ?
?

Main types
  * string
  * text
  * boolean
  * integer (or smallint, bigint)
  * float

Relationships / Associations
  * relation (a wizard will help you build the relation)
  * ManyToOne
  * OneToMany
  * ManyToMany
  * OneToOne

Array/Object Types
  * array (or simple_array)
  * json (or json_array)
  * object
  * binary
  * blob

Date/Time Types
  * datetime (or datetime_immutable)
  * datetimetz (or datetimetz_immutable)
  * date (or date_immutable)
  * time (or time_immutable)
  * dateinterval

Other Types
  * decimal
  * guid

 Field type (enter ? to see all types) [string]:
 > OneToOne
OneToOne

 What class should this entity be related to?:
 > Shop
Shop

 Is the CustomBootstrapTheme.shop property allowed to be null (nul
lable)? (yes/no) [yes]:
 > no

 Do you want to add a new property to Shop so that you can access/update Cus
tomBootstrapTheme objects from it - e.g. $shop->getCustomBootstrapTheme()? (yes/n
o) [no]:
 > yes

 A new property will also be added to the Shop class so that you can access the related CustomBo
otstrapTheme object from it.

 New field name inside Shop [customBootstrapTheme]:
 >

 updated: src/Entity/CustomBootstrapTheme.php
 updated: src/Entity/Shop.php

The resulting src/Entity/CustomBootstrapTheme.php file looks ok:

<?php

namespace App\Entity;

use Doctrine\ORM\Mapping as ORM;

/**
 * @ORM\Entity(repositoryClass="App\Repository\CustomBootstrapThemeRepository")
 */
class CustomBootstrapTheme
{
    /**
     * @ORM\Id()
     * @ORM\GeneratedValue()
     * @ORM\Column(type="integer")
     */
    private $id;

    /**
     * @ORM\OneToOne(targetEntity="App\Entity\Shop", inversedBy="customBootstrapTheme", cascade={"persist", "remove"})
     * @ORM\JoinColumn(nullable=false)
     */
    private $shop;

    public function getId()
    {
        return $this->id;
    }

    public function getShop(): ?Shop
    {
        return $this->shop;
    }

    public function setShop(Shop $shop): self
    {
        $this->shop = $shop;

        return $this;
    }
}

But here is where it modified the existing src/Entity/Shop.php file:

...
 private $__EXTRA__LINE;
  /**
   * @ORM\OneToOne(targetEntity="App\Entity\CustomBootstrapTheme", mappedBy="shop", cascade={"persist", "remove"})
   */
  private $customBootstrapTheme;
...

  private $__EXTRA__LINE;
  public function getCustomBootstrapTheme(): ?CustomBootstrapTheme
  {
      return $this->customBootstrapTheme;
  }
  private $__EXTRA__LINE;
  public function setCustomBootstrapTheme(CustomBootstrapTheme $customBootstrapTheme): self
  {
      $this->customBootstrapTheme = $customBootstrapTheme;
      $__EXTRA__LINE;
      // set the owning side of the relation if necessary
      if ($this !== $customBootstrapTheme->getShop()) {
          $customBootstrapTheme->setShop($this);
      }
      $__EXTRA__LINE;
      return $this;
  }

In my composer.json:

...
"repositories": [
    {
      "type":"vcs",
      "url":"https://github.com/ChanOmegaWebDesign/FOSMessageBundle.git"
    }

  ],
  "require": {
    "php": "^7.1.3",
    "ext-iconv": "*",
    "api-platform/api-pack": "^1.0",
    "friendsofsymfony/message-bundle": "dev-symfony4",
    "friendsofsymfony/user-bundle": "dev-master",
    "sensio/framework-extra-bundle": "^5.1",
    "symfony/console": "^4.0",
    "symfony/finder": "^4.0",
    "symfony/flex": "^1.0",
    "symfony/form": "^4.0",
    "symfony/framework-bundle": "^4.0",
    "symfony/lts": "^4@dev",
    "symfony/maker-bundle": "^1.0",
    "symfony/orm-pack": "^1.0",
    "symfony/profiler-pack": "^1.0",
    "symfony/security-csrf": "^4.0",
    "symfony/swiftmailer-bundle": "^3.1",
    "symfony/templating": "^4.0",
    "symfony/twig-bundle": "^4.0",
    "symfony/var-dumper": "^4.0",
    "symfony/webpack-encore-pack": "^1.0",
    "symfony/yaml": "^4.0",
    "twbs/bootstrap": "4.0.0"
  },
  "require-dev": {
    "symfony/dotenv": "^4.0",
    "symfony/web-server-bundle": "^4.0"
  },
  "config": {
    "preferred-install": {
      "*": "dist"
    },
    "sort-packages": true
  },
...

In the browser:

(1/1) FatalErrorExceptionCompile Error: Cannot redeclare App\Entity\Shop::$__EXTRA__LINE
--
in Shop.php line 391

Removing the 5 lines of code containing $__EXTRA__LINE clears the error, and everything then works as expected.

ChanOmegaWebDesign commented 6 years ago

It looks like the method \Symfony\Bundle\MakerBundle\Util\ClassSourceManipulator::addSingularRelation needs to call the method \Symfony\Bundle\MakerBundle\Util\ClassSourceManipulator::updateSourceCodeFromNewStmts addSingularRelation definitely was called, and updateSourceCodeFromNewStmts is the only method to remove the empty line place holders.

weaverryan commented 6 years ago

Reopening. Thanks for the info - we’ll see if we can repeat. Definitely a bug.

Rmy5 commented 6 years ago

I have the same issue. Can provide info if necessary.

Rmy5 commented 6 years ago

I wonder if this has to do with conflicting line endings CRLF vs LF

severfire commented 6 years ago

i do have similar issue. In the past i did setup my phpStorm to change space indent to tab indent... maybe it is related, dunno about if it is changing also new lines to something more optimized... anyhow, when using maker... i get $EXTRALINE issue :-/ and it looks the same as first example...

lgeorget commented 6 years ago

Maybe it's indeed related to tab indent. I use tabs for indentation too and I face the same issue.

Oros42 commented 6 years ago

Same error for me. symfony ^3.4 symfony/maker-bundle ^1.5 indent : 2 spaces OS : GNU\Linux

Entity : Test.php Before

<?php

namespace App\Entity;

use Symfony\Component\Validator\Constraints as Assert;
use Doctrine\ORM\Mapping as ORM;

/**
 * @ORM\Entity(repositoryClass="App\Repository\TestRepository")
 */
class Test {

  /**
   * @var integer
   *
   * @ORM\Column(name="id_test", type="bigint")
   * @ORM\Id
   * @ORM\GeneratedValue(strategy="AUTO")
   */
  private $id;

  /**
   * @var string
   *
   * @ORM\Column(name="plop", type="string", length=255, nullable=true)
   */
  private $plop;
}

After php bin/console make:entity --regenerate App :

<?php

namespace App\Entity;

use Symfony\Component\Validator\Constraints as Assert;
use Doctrine\ORM\Mapping as ORM;

/**
 * @ORM\Entity(repositoryClass="App\Repository\TestRepository")
 */
class Test {

  /**
   * @var integer
   *
   * @ORM\Column(name="id_test", type="bigint")
   * @ORM\Id
   * @ORM\GeneratedValue(strategy="AUTO")
   */
  private $id;

  /**
   * @var string
   *
   * @ORM\Column(name="plop", type="string", length=255, nullable=true)
   */
  private $plop;
  private $__EXTRA__LINE;
  public function getId(): ?int
  {
      return $this->id;
  }
  private $__EXTRA__LINE;
  public function getPlop(): ?string
  {
      return $this->plop;
  }
  private $__EXTRA__LINE;
  public function setPlop(?string $plop): self
  {
      $this->plop = $plop;
      $__EXTRA__LINE;
      return $this;
  }

}
javiereguiluz commented 6 years ago

I've tested this with MakerBundle v1.5.0 and I can't reproduce it. I've tested it with 4-whitespace tabs, 2-whitespace tabs, tabulator tabs, Windows-ending lines, macOS-ending lines, etc. I run ./bin/console make:entity --regenerate App and $__EXTRA__LINE; and private $__EXTRA__LINE; never appear 🤔

Rmy5 commented 6 years ago

Maybe it's PhpStorm related? I have the issue and I'm using PhpStorm 2018.1.6.

Oros42 commented 6 years ago

@javiereguiluz :-/

@Rmy5 I use sublimetext

javiereguiluz commented 6 years ago

I did the tests with Sublime ... but I also use PhpStorm and I've never seen this issue. could you please share the exact steps that you can follow to reproduce the error? Thanks!

Ex3v commented 6 years ago

Also having this issue. Fresh Symfony 4.1 installation, makerBundle @1.5.0.

I added private fields and annotated, then ran bin/console make:entity --regenerate and provided classname. I use OS X 10.11, php 7.1.12, zsh Results:

/**
 * @ORM\Entity()
 * @ORM\Table(name="user_basic_data")
 */
class UserBasicData
{

    /**
     * @var string
     * @ORM\Column(type="string", name="description", nullable=true)
     */
    private $description;

    /**
     * @var string
     * @ORM\Column(type="string", name="ninja_skills", nullable=true)
     */
    private $ninjaSkills;
 private $__EXTRA__LINE;
 public function getDescription(): ?string
 {
     return $this->description;
 }
 private $__EXTRA__LINE;
 public function setDescription(?string $description): self
 {
     $this->description = $description;
     $__EXTRA__LINE;
     return $this;
 }
 private $__EXTRA__LINE;
 public function getNinjaSkills(): ?string
 {
     return $this->ninjaSkills;
 }
 private $__EXTRA__LINE;
 public function setNinjaSkills(?string $ninjaSkills): self
 {
     $this->ninjaSkills = $ninjaSkills;
     $__EXTRA__LINE;
     return $this;
 }

}
Jeordy commented 6 years ago

Yep, me too on a macOS 10.12.6 Sierra, PHP7.1 with Zend opCache (homebrew). It doesn't matter if I use --regenerate from the terminal app or the terminal pane in PhpStorm. Both result in the $__EXTRA_LINE; bug. Removing these lines solves the issue, but it's quite time-consuming removing these lines manually over and over again in large entity files.

stof commented 6 years ago

please provide the file both before and after running the command. If you provide only the output file, this does not allow reproducing the issue.

Jeordy commented 6 years ago

Hi stof,

I've uploaded two files:

TestEntityBefore.txt TestEntityAfter.txt

Jeordy commented 6 years ago

@stof I think I found the bug;

The ClassSourceManipulator removes all the fake placeholders, which are: private $EXTRALINE, etc. at line 662. In str_replace there are 4 whitespaces in front of ' $__EXTRA_LINE' and ' $EXTRALINE', but not 'use __EXTRA_LINE'.

When I execute: ./bin/console make:entity --regenerate and insert the entity, all getters and setters have just 1 whitespace in front off them, in stead of 4.

I changed the str_replace on line 662 into: $newCode = str_replace([' private $EXTRALINE;', 'use EXTRALINE;', ' $EXTRALINE;'], '', $newCode); and now the code works perfectly. However the getters and setters only have 1 whitespace, where I would prefer 4, because it looks better ;-)

maisoui commented 6 years ago

Hi, same error here with fresh project setup with Symfony 4.1. Regards

Ex3v commented 6 years ago

Sounds promising! @stof what do you think? Or maybe @Jeordy would you be able to create new PR that would solve the issue?

Jeordy commented 6 years ago

@Ex3v I really would like to create a new PR, but my 'solution' only works when the --regenerate produces the 1 whitespace, as where many people do not have this issue (I think it's macOS related).

My guess it has something to do with the escape sequence when creating the getters and setters. When creating a text file, you can define a tab delimiter with \t. I cannot find any \t used in any file of the maker bundle.

Jeordy commented 6 years ago

@weaverryan @stof What I found is that the ClassSourceManipulator uses the nikic/php-parser for the indentation. When I follow the code, it takes me to: vendor/nikic/php-parser/lib/PhpParser/internal/TokenStream.php. In this class there is a private function calcIndentMap(), which calculates the indentation. There, the new getters and setters get an indentation of 1, which means 1 whitespace. I've changed the function and replace the 1 with 4, so the code looks like this:

vendor/nikic/php-parser/lib/PhpParser/internal/TokenStream.php:

private function calcIndentMap() {
        $indentMap = [];
        $indent = 0;

        foreach ($this->tokens as $token) {
            $indentMap[] = $indent;

            if ($token[0] === \T_WHITESPACE) {
                $content = $token[1];
                $newlinePos = \strrpos($content, "\n");
                if (false !== $newlinePos) {
                    $indent = \strlen($content) - $newlinePos - 1;

                    // ADDED: Change indentation to 4 spaces
                    if ($indent == 1) $indent = 4;
                }
            }
        }

        // Add a sentinel for one past end of the file
        $indentMap[] = $indent;

        return $indentMap;
    }

The --regenerate works as expected, but my guess is that there is a function that adds the 1 whitespace, where it needs to be 4.

Wormaster commented 6 years ago

Quick&dirty fix: \Symfony\Bundle\MakerBundle\Util\ClassSourceManipulator

replace or comment line 662: newCode = str_replace([' private $EXTRALINE;', 'use EXTRALINE;', ' $EXTRALINE;'], '', $newCode); with: $newCode = preg_replace('/^\s{0,}(private\s|use\s)?\$?EXTRALINE;/m', '', $newCode);

Indent will be 1 space but it will produce working code.

weaverryan commented 6 years ago

@Jeordy Thanks for checking into this! It's super frustrating because none of the main contributors can repeat the bug. But, since you can and are motivated, I'm sure we can fix it!

I think we can find an easy fix for this. The WHOLE point of the problematic code (the str_replace at line 662) is to replace that line with an empty line. So, I think we can do something similar to the "Quick & dirty" fixes like @Wormaster suggested, just a little bit less dirty ;).

Basically, all we need to do, for example, is search for private $__EXTRA__LINE;, then replace that ENTIRE line with empty quotes. We should be able to do that with some regex, or dirtier "searching" of the beginning and ending line positions. It looks like there are several solutions that are almost there: we just need to replace the entire line with empty quotes so that there are zero spaces.

Another solution is to use one of the "dirty" solutions proposed by @Wormaster or you @Jeordy. But then do a second operation: look at the entire source file, and, if you find any lines that only have whitespace, remove that whitespace (so the line is empty). There really should be an easier solution - but if this works, it's fine for me - we have plenty of tests to make sure we get the exact right code.

If anyone finds something that fully works, I'd love to see a PR - no matter how dirty it may look ;).

Cheers!

Wormaster commented 6 years ago

@weaverryan The main problem is in TokenStream class because it does not recognise TAB indent. I found a way to reproduce the problem: IDE: PhpStorm 1) You need to change indent for PHP scripts to TABs 2) Remove all generated methods from some entity class (in my case "App\Entity\User") 3) Use Auto-Indent funnction on this class 4) run - php bin/console m:e --regenerate "App\Entity\User" 5) That's it

weaverryan commented 6 years ago

Yep, I can repeat it now - thanks @Wormaster! Working on a fix...

weaverryan commented 6 years ago

Fix is up at #238!