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.

Sign up for GitHub

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

Jump to bottom

CheckUnusedViews don't check method view in MailMessage class #2063

Closed
koraga opened this issue Oct 10, 2024 · 4 comments · Fixed by #2064
Closed

CheckUnusedViews don't check method view in MailMessage class #2063

koraga opened this issue Oct 10, 2024 · 4 comments · Fixed by #2064
Labels
bug Something isn't working

Comments

@koraga
Copy link
Contributor

koraga commented Oct 10, 2024

  • Larastan Version: 2.9.8
  • Laravel Version: 11.26.0

Description

I am using views in my notifications with the MailMessage class, but phpstan doesn't recognize that these views are being used there and throws an error with the rule checkUnusedViews: true.

Laravel code where the issue was found

class TournamentConfirmationRegistration extends Notification
{
    /**
     * @return non-empty-list<string>
     */
    public function via(): array
    {
        return [MailChannel::class];
    }

    public function toMail(): MailMessage
    {
        return (new MailMessage())->view('mails.tournament-confirmation-registration');
    }
}

error:

 ------ ---------------------------------------------------------------------- 
  Line   resources/views/mails/tournament-confirmation-registration.blade.php  
 ------ ---------------------------------------------------------------------- 
  0      This view is not used in the project.                                 
 ------ ----------------------------------------------------------------------
@koraga koraga added the bug Something isn't working label Oct 10, 2024
@calebdw
Copy link
Contributor

calebdw commented Oct 10, 2024

I'm not sure that we are collecting views from MailMessage calls (only Mailable calls):

larastan/src/Rules/UnusedViewsRule.php

Lines 50 to 57 in b0bfa09

$usedViews = collect([
$node->get(UsedViewFunctionCollector::class),
$node->get(UsedEmailViewCollector::class),
$node->get(UsedViewMakeCollector::class),
$node->get(UsedViewFacadeMakeCollector::class),
$node->get(UsedRouteFacadeViewCollector::class),
$this->viewsUsedInOtherViews,
])->flatten()->unique()->toArray();

@koraga
Copy link
Contributor Author

koraga commented Oct 10, 2024 •
edited
Loading

I'm not sure that we are collecting views from MailMessage calls (only Mailable calls):

larastan/src/Rules/UnusedViewsRule.php

Lines 50 to 57 in b0bfa09

$usedViews = collect([
$node->get(UsedViewFunctionCollector::class),
$node->get(UsedEmailViewCollector::class),
$node->get(UsedViewMakeCollector::class),
$node->get(UsedViewFacadeMakeCollector::class),
$node->get(UsedRouteFacadeViewCollector::class),
$this->viewsUsedInOtherViews,
])->flatten()->unique()->toArray();

I found this too

@koraga
Copy link
Contributor Author

koraga commented Oct 10, 2024

I can contribute and add a new Collector for MailMessage

@calebdw
Copy link
Contributor

calebdw commented Oct 10, 2024 •
edited
Loading

All that should be needed is to also check that MailMessage is a super type

larastan/src/Collectors/UsedEmailViewCollector.php

Lines 45 to 47 in b0bfa09

if (! (new ObjectType(Mailable::class))->isSuperTypeOf($scope->getType($class))->yes()) {
return null;
} 

$type = $scope->getType($class);
 if (
    ! (new ObjectType(Mailable::class))->isSuperTypeOf($type)->yes()
    && ! (new ObjectType(MailMessage::class))->isSuperTypeOf($type)->yes()
) { 
     return null; 
 }

@koraga koraga mentioned this issue Oct 10, 2024
Merged
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Support MailMessage in UsedEmailViewCollector koraga/larastan
2 participants
@calebdw @koraga