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

array_keys() issue only with phar version #2858

Closed
Rarst opened this issue Feb 21, 2020 · 8 comments · Fixed by #10748
Closed

array_keys() issue only with phar version #2858

Rarst opened this issue Feb 21, 2020 · 8 comments · Fixed by #10748

Comments

@Rarst
Copy link
Contributor

Rarst commented Feb 21, 2020

For this code (a bit too much going on to reproduce in sandbox):

	/**
	 * @return Stopwatch_Record[]
	 */
	public function get_records(): array {

		/** @psalm-var array<string, StopwatchEvent> $events */
		$events = $this->stopwatch->getSectionEvents( '__root__' );

		/** @var string $name */
		foreach ( array_keys( $events ) as $name ) {
			if ( $this->stopwatch->isStarted( $name ) ) {
				unset( $events[ $name ] );
			}
		}

		return array_map( [ $this, 'transform' ], array_keys( $events ), $events );
	}

	/**
	 * @param string         $name  Event name.
	 * @param StopwatchEvent $event Stopwatch event instance.
	 *
	 * @return Stopwatch_Record
	 */
	protected function transform( string $name, StopwatchEvent $event ): Stopwatch_Record {

		return new Stopwatch_Record( $name, $event );
	}

https://github.com/Rarst/laps/blob/168a226859dade7b431d2d69ba751fb3667ee146/src/Record/Collector/Stopwatch_Collector.php#L49-L76

Getting this error:

ERROR: InvalidScalarArgument - src\Record\Collector\Stopwatch_Collector.php:64:21 - First parameter of closure passed to function array_map expects string, int|string provided
return array_map( [ $this, 'transform' ], array_keys( $events ), $events );

However array_keys( $events ) should only contain/pass strings since $events is documented to only have string keys above array<string, StopwatchEvent>.

@psalm-github-bot
Copy link

Hey @Rarst, can you reproduce the issue on https://psalm.dev ?

@Rarst
Copy link
Contributor Author

Rarst commented Feb 21, 2020

Hmm, seems to not happen on master, but I am not finding recent issues about array_keys()...

@Rarst Rarst closed this as completed Feb 21, 2020
@Rarst Rarst changed the title array_keys() doesn't pick up when input keys documented to be only string array_keys() issue only with phar version Mar 10, 2020
@Rarst
Copy link
Contributor Author

Rarst commented Mar 10, 2020

Okay, this got stranger — I can reproduce this, but only with phar version, even if it has exact same commit version as git checkout. Here is running both on current master of the repo (psalm is phar, psalm-dev is git checkout):

c:\server\www\dev\wp-content\plugins\laps>psalm --version & psalm-dev --version &&  psalm & psalm-dev
Psalm 3.9.5@0cfe565d0afbcd31eadcc281b9017b5692911661
Psalm 3.x-dev@0cfe565d0afbcd31eadcc281b9017b5692911661
Scanning files...
Analyzing files...

░��░��░��░��░��░��░��░��░��░��░��░��░��░��░��░��░��░��░��░��░��░��░��░��░��░��EE░��░��░��░��░��

ERROR: InvalidScalarArgument - src\Record\Collector\Sql_Collector.php:68:39 - First parameter of closure passed to function array_map expects int, int|string provided
                $records = array_filter( array_map( [ $this, 'transform' ], array_keys( $wpdb->queries ), $wpdb->queries ) );


ERROR: InvalidScalarArgument - src\Record\Collector\Stopwatch_Collector.php:64:21 - First parameter of closure passed to function array_map expects string, int|string provided
                return array_map( [ $this, 'transform' ], array_keys( $events ),
$events );


------------------------------
2 errors found
------------------------------

Checks took 4.41 seconds and used 56.625MB of memory
Psalm was able to infer types for 95.1049% of the codebase
Scanning files...
Analyzing files...

░��░��░��░��░��░��░��░��░��░��░��░��░��░��░��░��░��░��░��░��░��░��░��░��░��░��░��░��░��░��░��░��░��

------------------------------
No errors found!
------------------------------

Checks took 4.69 seconds and used 62.938MB of memory
Psalm was able to infer types for 96.1583% of the codebase

@Rarst Rarst reopened this Mar 10, 2020
@Rarst
Copy link
Contributor Author

Rarst commented Mar 10, 2020

Possibly related #2953 since on Windows git defaults to checkout with Windows line endings (but commit with Unix line endings).

So phar copy would have original Unix line endings, but git copy would have them changed to Windows ones.

@Rarst
Copy link
Contributor Author

Rarst commented Mar 10, 2020

I tried replacing line endings to Unix ones in source PHP files, but still get the issue. :\

@veewee
Copy link
Contributor

veewee commented Jul 17, 2020

I've created a POC to reproducte the error:

It happens while using the phar version on windows.
For mac + linux it works fine.
(Haven't tested the regular composer version)

Other functions/types that fail in a similar way only on windows:

  • array_flip
  • pathinfo (with second param string)
  • Iterable<K, V> invalid amount of templates
  • Generator<A,Promise<X>,C,D> invalid amount of templates

Let me know if I can do anything to help solve this issue!

@weirdan
Copy link
Collaborator

weirdan commented Feb 25, 2024

I suspect it's fixed in #10748. @veewee can you try with this phar: https://output.circle-artifacts.com/output/job/2b519c3a-9271-40fe-abfa-7e6f8a894f5c/artifacts/0/build/psalm.phar

PS: yes, it took me 4 years to reboot to Windows.
Edit: fixed phar link

@veewee
Copy link
Contributor

veewee commented Feb 26, 2024

Seems to work!
I Uploaded the phar and the action is green:

Screenshot 2024-02-26 at 09 07 32

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants