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

[3.x] Fix return type #2140

Merged
merged 4 commits into from
Feb 10, 2023
Merged

[3.x] Fix return type #2140

merged 4 commits into from
Feb 10, 2023

Conversation

aksiome
Copy link
Contributor

@aksiome aksiome commented Feb 9, 2023

A quick fix to silence IDE errors

I've noticed that setter methods used the self return type when returning $this. I believe it's best to return static now. This way users can extend classes without getting errors when chaining method calls.

I also removed the slugs mixin from the model interface since it is supposed to be an optional trait. Noticed it was added with the caching route fix but I'm not entirely sure why.

@what-the-diff
Copy link

what-the-diff bot commented Feb 9, 2023

  • Removed HasSlug trait from TwillModelContract
  • Changed return type of newInstance() method in Block class to static instead of self
  • Changed return types for default(), label(), note(), required() and disabled methods in BaseFormField class to static instead of self
  • Added missing use statement for A17\Twill\Models\Behaviors namespace at the top of BlockEditor, Browser, Checkbox, DatePicker and Files classes
  • Replaced all occurrences (except one) with "static" keyword as a replacement for "self" keyword when defining return types on public functions inside these files:
  • Changed the return type of all methods to static
  • Removed unused imports in Repeater and MultiSelect fields
  • Changed the return type of some methods from self to static
  • Added a missing use statement in Form class
  • Changed the return type of all methods in QuickFilter and TwillBaseFilter to static
  • Added a default sort direction for TableColumns (ASC)

@codecov
Copy link

codecov bot commented Feb 9, 2023

Codecov Report

Base: 59.92% // Head: 59.92% // No change to project coverage 👍

Coverage data is based on head (4ea3e4a) compared to base (aa6d0f2).
Patch coverage: 40.00% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff            @@
##                3.x    #2140   +/-   ##
=========================================
  Coverage     59.92%   59.92%           
  Complexity     3898     3898           
=========================================
  Files           303      303           
  Lines         15014    15014           
=========================================
  Hits           8997     8997           
  Misses         6017     6017           
Impacted Files Coverage Δ
src/Services/Forms/Fields/Checkbox.php 27.77% <0.00%> (ø)
src/Services/Forms/Fields/Input.php 29.41% <0.00%> (ø)
src/Services/Forms/Fields/Map.php 29.41% <0.00%> (ø)
src/Services/Forms/Fields/Medias.php 20.58% <0.00%> (ø)
src/Services/Forms/Fields/MultiSelect.php 25.00% <0.00%> (ø)
...ervices/Forms/Fields/Traits/CanHaveButtonOnTop.php 0.00% <0.00%> (ø)
src/Services/Forms/Fields/Traits/CanReorder.php 0.00% <0.00%> (ø)
src/Services/Forms/Fields/Traits/HasBorder.php 0.00% <0.00%> (ø)
src/Services/Forms/Fields/Traits/HasFieldNote.php 0.00% <0.00%> (ø)
src/Services/Forms/Fields/Traits/HasMax.php 0.00% <0.00%> (ø)
... and 31 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@ifox
Copy link
Member

ifox commented Feb 9, 2023

Awesome @aksiome, we were actually discussing addressing this with @haringsrob a couple days ago. And @joyceverheije was looking into it to to fix PHPstan issues.

Thank you for working on this!

@CLAassistant
Copy link

CLAassistant commented Feb 9, 2023

CLA assistant check
All committers have signed the CLA.

@haringsrob
Copy link
Contributor

Hey @aksiome this was indeed on my list too long! Thanks for fixing, the capsule phpcs issue is fixed in another pr so this is good to go!

@haringsrob haringsrob merged commit 4c2ae8c into area17:3.x Feb 10, 2023
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.

4 participants