Indicate file on TypeInferenceTestCase validation errors #3166

staabm · 2024-06-19T07:15:43Z

was just hunting down a bug in one of my PRs, but wasn't able to find the offending file, because the error message did not indicate which file the error related to

before this PR:

There was 1 error:

1) Error
The data provider specified for PHPStan\Analyser\NodeScopeResolverTest::testFileAsserts is invalid.
PHPUnit\Framework\AssertionFailedError: Expected type must be a literal string, string given on line 78.
/Users/staabm/workspace/phpstan-src/src/Testing/TypeInferenceTestCase.php:166
/Users/staabm/workspace/phpstan-src/src/Node/ClassStatementsGatherer.php:128
/Users/staabm/workspace/phpstan-src/src/Analyser/NodeScopeResolver.php:650
/Users/staabm/workspace/phpstan-src/src/Analyser/NodeScopeResolver.php:759
/Users/staabm/workspace/phpstan-src/src/Analyser/NodeScopeResolver.php:3938
/Users/staabm/workspace/phpstan-src/src/Analyser/NodeScopeResolver.php:2124
/Users/staabm/workspace/phpstan-src/src/Analyser/NodeScopeResolver.php:758
/Users/staabm/workspace/phpstan-src/src/Analyser/NodeScopeResolver.php:339
/Users/staabm/workspace/phpstan-src/src/Analyser/NodeScopeResolver.php:649
/Users/staabm/workspace/phpstan-src/src/Analyser/NodeScopeResolver.php:339
/Users/staabm/workspace/phpstan-src/src/Analyser/NodeScopeResolver.php:835
/Users/staabm/workspace/phpstan-src/src/Analyser/NodeScopeResolver.php:339
/Users/staabm/workspace/phpstan-src/src/Analyser/NodeScopeResolver.php:803
/Users/staabm/workspace/phpstan-src/src/Analyser/NodeScopeResolver.php:296
/Users/staabm/workspace/phpstan-src/src/Testing/TypeInferenceTestCase.php:92
/Users/staabm/workspace/phpstan-src/src/Testing/TypeInferenceTestCase.php:145
/Users/staabm/workspace/phpstan-src/src/Testing/TypeInferenceTestCase.php:271
/Users/staabm/workspace/phpstan-src/tests/PHPStan/Analyser/NodeScopeResolverTest.php:17

after this PR:

1) Error
The data provider specified for PHPStan\Analyser\NodeScopeResolverTest::testFileAsserts is invalid.
PHPUnit\Framework\AssertionFailedError: Expected type must be a literal string, string given in tests/PHPStan/Analyser/nsrt/param-closure-this.php on line 78.
/Users/staabm/workspace/phpstan-src/src/Testing/TypeInferenceTestCase.php:166
/Users/staabm/workspace/phpstan-src/src/Node/ClassStatementsGatherer.php:128
/Users/staabm/workspace/phpstan-src/src/Analyser/NodeScopeResolver.php:650
/Users/staabm/workspace/phpstan-src/src/Analyser/NodeScopeResolver.php:759
/Users/staabm/workspace/phpstan-src/src/Analyser/NodeScopeResolver.php:3938
/Users/staabm/workspace/phpstan-src/src/Analyser/NodeScopeResolver.php:2124
/Users/staabm/workspace/phpstan-src/src/Analyser/NodeScopeResolver.php:758
/Users/staabm/workspace/phpstan-src/src/Analyser/NodeScopeResolver.php:339
/Users/staabm/workspace/phpstan-src/src/Analyser/NodeScopeResolver.php:649
/Users/staabm/workspace/phpstan-src/src/Analyser/NodeScopeResolver.php:339
/Users/staabm/workspace/phpstan-src/src/Analyser/NodeScopeResolver.php:835
/Users/staabm/workspace/phpstan-src/src/Analyser/NodeScopeResolver.php:339
/Users/staabm/workspace/phpstan-src/src/Analyser/NodeScopeResolver.php:803
/Users/staabm/workspace/phpstan-src/src/Analyser/NodeScopeResolver.php:296
/Users/staabm/workspace/phpstan-src/src/Testing/TypeInferenceTestCase.php:92
/Users/staabm/workspace/phpstan-src/src/Testing/TypeInferenceTestCase.php:145
/Users/staabm/workspace/phpstan-src/src/Testing/TypeInferenceTestCase.php:273
/Users/staabm/workspace/phpstan-src/tests/PHPStan/Analyser/NodeScopeResolverTest.php:17

note the added filename in the error message

src/Testing/TypeInferenceTestCase.php

phpstan-bot · 2024-06-19T08:01:03Z

This pull request has been marked as ready for review.

src/Testing/TypeInferenceTestCase.php

staabm · 2024-06-19T09:18:05Z

I will finalize this PR when back at my windows laptop next week

phpstan-bot · 2024-07-09T14:44:02Z

This pull request has been marked as ready for review.

ondrejmirtes · 2024-07-10T07:11:42Z

src/Testing/TypeInferenceTestCase.php

 					$functionName,
+					$pathHelper->getRelativePath(strtr($file, '\\', '/')),


We don't need to change the backslashes here I think. And if you wanted to, we have FileHelper::normalizePath() for that. You can run this function on both sides here (implementation & tests).

the currentWorkingDirectory in

phpstan-src/src/File/SimpleRelativePathHelper.php

Line 19 in 519982c

if ($this->currentWorkingDirectory !== '' && str_starts_with($filename, $this->currentWorkingDirectory)) {

is normalized to forward slashes.

when not doing strtr($file, '\\', '/') we pass on windows a path with backslashes and this makes the path not getting returned relative.

just tried FileHelper::normalize instead of strtr($file, '\\', '/'): it will not turn the slashes in the correct form to make $pathHelper->getRelativePath return a proper relative path

Why wouldn't it? It works everywhere else in PHPStan's codebase.

I don't think so.

in other places we also do the \\ -> / dance to make it work:

phpstan-src/tests/e2e/ResultCacheEndToEndTest.php

Lines 194 to 199 in 494b7b0

private function relativizePath(string $path): string

{

$path = str_replace('\\', '/', $path);

$helper = new SimpleRelativePathHelper(str_replace('\\', '/', __DIR__ . '/PHP-Parser'));

return $helper->getRelativePath($path);

}

but I found a way to make it work.

ondrejmirtes · 2024-07-10T13:07:10Z

src/Testing/TypeInferenceTestCase.php

@@ -143,8 +145,10 @@ public function assertFileAsserts(
 	 */
 	public static function gatherAssertTypes(string $file): array
 	{
+		$pathHelper = new SimpleRelativePathHelper(dirname(__DIR__, 2));


This doesn't make sense as it will be somewhere deep in the PHAR, for users that use TypeInferenceTestCase outside of phpstan-src.

last try in the latest git push. if this doesn't work for you I will close this PR.

I committed my vision how it should have been done: 0e53118

ondrejmirtes · 2024-07-11T11:53:43Z

Thank you.

As a next step we could stop using SimpleRelativePathHelper in more places in tests and start using SystemAgnosticSimpleRelativePathHelper instead. It could find more issues like it did here in PathConstantsTest.

staabm force-pushed the lines branch from d3b9fa4 to df9fd94 Compare June 19, 2024 07:42

ondrejmirtes requested changes Jun 19, 2024

View reviewed changes

src/Testing/TypeInferenceTestCase.php Outdated Show resolved Hide resolved

staabm force-pushed the lines branch from 4003a16 to 805f0c7 Compare June 19, 2024 07:56

staabm marked this pull request as ready for review June 19, 2024 08:00

ondrejmirtes requested changes Jun 19, 2024

View reviewed changes

src/Testing/TypeInferenceTestCase.php Outdated Show resolved Hide resolved

staabm marked this pull request as draft June 19, 2024 09:18

clxmstaab force-pushed the lines branch from df148d6 to 58ec80c Compare July 9, 2024 14:30

staabm marked this pull request as ready for review July 9, 2024 14:43

ondrejmirtes requested changes Jul 10, 2024

View reviewed changes

clxmstaab force-pushed the lines branch 2 times, most recently from 45b7661 to afa4cb9 Compare July 10, 2024 12:46

ondrejmirtes requested changes Jul 10, 2024

View reviewed changes

staabm and others added 14 commits July 11, 2024 13:17

Indicate file on TypeInferenceTestCase validation errors

4c1278e

fix

65432b4

minimize IO

e6b7fc6

use current working directory

6bb1121

take path helper from the container

16a7497

fix

25d3d82

force SimpleRelativePathHelper

0603d41

fix windows compat

775d016

cs

0cfc896

simplify

f07d6ad

cs

c68239a

fix leftover

f1632c6

back to square 1

dcca482

fix

0e53118

ondrejmirtes force-pushed the lines branch from fc4b2c0 to 0e53118 Compare July 11, 2024 11:25

Introduce SystemAgnosticSimpleRelativePathHelper

908f705

ondrejmirtes force-pushed the lines branch from e8bce06 to 908f705 Compare July 11, 2024 11:36

Fix test

d8acd25

ondrejmirtes force-pushed the lines branch from 7a7d0f2 to d8acd25 Compare July 11, 2024 11:47

ondrejmirtes merged commit 18706de into phpstan:1.11.x Jul 11, 2024
449 of 452 checks passed

staabm deleted the lines branch July 11, 2024 12:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Indicate file on TypeInferenceTestCase validation errors #3166

Indicate file on TypeInferenceTestCase validation errors #3166

staabm commented Jun 19, 2024 •

edited

Loading

phpstan-bot commented Jun 19, 2024

staabm commented Jun 19, 2024

phpstan-bot commented Jul 9, 2024

ondrejmirtes Jul 10, 2024

staabm Jul 10, 2024

staabm Jul 10, 2024 •

edited

Loading

ondrejmirtes Jul 10, 2024

staabm Jul 10, 2024

ondrejmirtes Jul 10, 2024

staabm Jul 10, 2024

ondrejmirtes Jul 11, 2024

ondrejmirtes commented Jul 11, 2024

		$functionName,
		$pathHelper->getRelativePath(strtr($file, '\\', '/')),

	private function relativizePath(string $path): string
	{
	$path = str_replace('\\', '/', $path);
	$helper = new SimpleRelativePathHelper(str_replace('\\', '/', __DIR__ . '/PHP-Parser'));
	return $helper->getRelativePath($path);
	}

Indicate file on TypeInferenceTestCase validation errors #3166

Indicate file on TypeInferenceTestCase validation errors #3166

Conversation

staabm commented Jun 19, 2024 • edited Loading

phpstan-bot commented Jun 19, 2024

staabm commented Jun 19, 2024

phpstan-bot commented Jul 9, 2024

ondrejmirtes Jul 10, 2024

Choose a reason for hiding this comment

staabm Jul 10, 2024

Choose a reason for hiding this comment

staabm Jul 10, 2024 • edited Loading

Choose a reason for hiding this comment

ondrejmirtes Jul 10, 2024

Choose a reason for hiding this comment

staabm Jul 10, 2024

Choose a reason for hiding this comment

ondrejmirtes Jul 10, 2024

Choose a reason for hiding this comment

staabm Jul 10, 2024

Choose a reason for hiding this comment

ondrejmirtes Jul 11, 2024

Choose a reason for hiding this comment

ondrejmirtes commented Jul 11, 2024

staabm commented Jun 19, 2024 •

edited

Loading

staabm Jul 10, 2024 •

edited

Loading