Skip to content

Conversation

@paulbalandan
Copy link
Member

Description
This PR proposes to change how we use the class_attributes_separation rule. Currently, we only activate changing the separation between class methods to always have a single blank line. This now includes class constants, properties, and trait uses in the picture by using this logic:

  1. If the const, property, or trait use has metadata (PHPDoc or Attribute) attached to it, it gets one line.
  2. If none, then no blank lines are enforced.
  3. If preceding class attribute has metadata while the current has none, there will still be a blank line.
class Foo
{
    /** @var Foo */
    public $bar;
+
    public $baz;
-
    public $qux;
}
  1. There are blank lines in between attributes:
class Foo
{
    use FooTrait;
    use BarTrait;
+
    public $bar;
    public $baz;
}

Checklist:

  • Securely signed commits
  • Conforms to style guide

@kenjis
Copy link
Member

kenjis commented Oct 6, 2021

I disagree with 2. If none, then no blank lines are enforced.

CI4 classes has a lot of properties, and there are many blank lines.
Blank lines are inserted with some intention (perhaps grouping). Removing empty lines
losts the intention.

Why do you want to remove blank lines?

@paulbalandan
Copy link
Member Author

I tend to agree it may lose the intention. However, some of the spacings are arbitrary, if not useless. I would love to keep the spacing if the intention for doing so is clearly evident in the code (by way of regular comments, perhaps) -- for example, the groupings made in https://github.com/codeigniter4/CodeIgniter4/blob/develop/system/Commands/Generators/Views/model.tpl.php is clear to the intention.

    // Dates
    protected $useTimestamps = false;
    protected $dateFormat    = 'datetime';
    protected $createdField  = 'created_at';
    protected $updatedField  = 'updated_at';
    protected $deletedField  = 'deleted_at';

    // Validation
    protected $validationRules      = [];
    protected $validationMessages   = [];
    protected $skipValidation       = false;
    protected $cleanValidationRules = true;

Otherwise, I'd argue those blank lines adds no value.

@paulbalandan paulbalandan force-pushed the class-attrs-separation-behavior branch 2 times, most recently from 265a769 to cc9b673 Compare October 7, 2021 07:22
Copy link
Member

@MGatner MGatner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

YES! This is great, this one has always bothered me.

@paulbalandan paulbalandan force-pushed the class-attrs-separation-behavior branch from cc9b673 to b25e916 Compare October 11, 2021 08:11
@paulbalandan paulbalandan merged commit cb714b5 into codeigniter4:develop Oct 11, 2021
@paulbalandan paulbalandan deleted the class-attrs-separation-behavior branch October 11, 2021 09:03
paulbalandan added a commit to CodeIgniter/coding-standard that referenced this pull request Oct 18, 2021
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

Successfully merging this pull request may close these issues.

3 participants