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

view-string not behaving as expected #1607

Closed
JayBizzle opened this issue Mar 31, 2023 · 7 comments
Closed

view-string not behaving as expected #1607

JayBizzle opened this issue Mar 31, 2023 · 7 comments

Comments

@JayBizzle
Copy link

  • Larastan Version: 2.5.1
  • --level used: 9

Description

Perhaps I am not understanding correctly how this should work, but from what I have read, if I do something like...

    /**
     * Display the data.
     *
     * @return \Illuminate\View\View
     */
    public function index()
    {
        $data = [];

        return view('this-view-does-not-exist', ['data' => $data]);
    }

...Larastan should pick this up an throw an error because the view does not exist.

This doesn't seem to be the case.

I tested this with a fresh installation of Laravel 10 and the latest version of Larastan and no errors are reported when a view is missing.

I'm sure this worked as expected in earlier versions of Larastan. 2.2.7 is what we have just upgraded from.

Or have I misunderstood how the view-string type works?

Thanks

@szepeviktor
Copy link
Collaborator

We should have more tests.
https://github.com/nunomaduro/larastan/blob/master/tests/Type/data/view-exists.php

@JayBizzle
Copy link
Author

So, are you saying it isn't working as expected?

@szepeviktor
Copy link
Collaborator

szepeviktor commented Mar 31, 2023

Larastan checks view calls with 1 string only.
https://github.com/nunomaduro/larastan/blob/11bdd132565d37e808e4ac985a580b492c2d8887/src/Types/ViewStringType.php#L31-L36

Could you try with return view('this-view-does-not-exist');?

@szepeviktor
Copy link
Collaborator

I do not understand how it works. I'm just a janitor 🧹

@JayBizzle
Copy link
Author

JayBizzle commented Mar 31, 2023

Could you try with return view('this-view-does-not-exist');?

Same result 😕

@NiclasvanEyk
Copy link

While working on my implementation on route-name-string in #1734, I ran into a similar problem when trying to re-use the same logic as implemented by the ViewStringType.

In ViewStringType.php only the accepts method is overriden. The acceptsWithReason method which seems to be the 'new' way of implementing custom types is still using the implementation from StringType, which contains the following statement:

        public function acceptsWithReason(Type $type, bool $strictTypes): AcceptsResult
	{
		if ($type instanceof self) {
			return AcceptsResult::createYes();
		}

		// ...

Components such as the RuleLevelHelper, which is e.g. used to check whether the argument type of a function call is accepted by the parameter type, only call acceptsWithReason. Therefore when checking if the string argument passed to view('non-existing-view'), ViewStringType::acceptsWithReason is called, which delegates to StringType::acceptsWithReason. Since the argument to view is a string, the check $type instanceof self yields true, so now every string is also accepted by parameters declared as view-string, leading to the problems described in this issue.

A possible solution is to implement the logic that currently lives in ViewString::accepts in a new overridden ViewString::acceptsWithReason, moving all TrinaryLogic to AcceptanceResult and delegate ViewString::accepts to ViewString::acceptsWithReason()->result, similar to how it is done in StringType::accepts.

NiclasvanEyk added a commit to NiclasvanEyk/larastan that referenced this issue Sep 9, 2023
@NiclasvanEyk NiclasvanEyk mentioned this issue Sep 9, 2023
2 tasks
@canvural
Copy link
Collaborator

I believe this was fixed in newer releases. Thank you!

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

No branches or pull requests

4 participants