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

feat: unused views collector rule #1423

Merged
merged 4 commits into from Nov 23, 2022
Merged

feat: unused views collector rule #1423

merged 4 commits into from Nov 23, 2022

Conversation

canvural
Copy link
Collaborator

@canvural canvural commented Oct 24, 2022

  • Added or updated tests
  • Documented user facing changes
  • Updated CHANGELOG.md

This PR adds a new rule that can find the unused views, using PHPStan's collector feature. It'll be an optional rule.

Some concerns

  • Collectors for this feature tries to find used views in 2 places currently: view('foo') calls. And $this->markdown('foo') in any Mailable class. Are there more places that we can check?
  • Currently there are false positives if a view is only used in another view. For example with @extends('foo') I'd say this is an edge case, and can be ignored by the user.

Todos

  • This error should not happen. The mentioned views are from the dummy Laravel project Testbench created, not from the actual package.
  • Support View::make('foo') and $viewFactory->make()
  • Support $this->view('foo') in Mailables

How to test it?

Run composer require --dev nunomaduro/larastan:dev-unused-views Then run your PHPStan analysis again.
And report here if there are any false positives or errors that we can fix!

@mad-briller
Copy link
Contributor

Are there more places that we can check?

Alongside the view facade, the View\Factory contract may be injected and $viewFactory->make() called instead, so may be worth checking there too

@canvural
Copy link
Collaborator Author

Thanks, updated the list.

@canvural canvural force-pushed the unused-views branch 3 times, most recently from 45ed09d to 45997d5 Compare November 1, 2022 08:01
@canvural canvural force-pushed the unused-views branch 10 times, most recently from d52d8a7 to 3cc105b Compare November 7, 2022 07:54
@canvural canvural force-pushed the master branch 2 times, most recently from 6e390bd to 325d2f3 Compare November 9, 2022 13:27
@canvural canvural marked this pull request as ready for review November 23, 2022 20:12
@canvural canvural merged commit 44fe71d into master Nov 23, 2022
@canvural canvural deleted the unused-views branch November 23, 2022 20:12
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.

None yet

2 participants