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

Fix property case in Navigation_Routing_Component #239

Merged
merged 2 commits into from
Feb 12, 2020
Merged

Fix property case in Navigation_Routing_Component #239

merged 2 commits into from
Feb 12, 2020

Conversation

szepeviktor
Copy link
Contributor

Discovered by @phpstan

@westonruter
Copy link
Collaborator

@szepeviktor Now that we've moved the repo over to the GoogleChromeLabs org, could you please sign the CLA, if you haven't done so already? Then please rebase the branch or merge the latest from master to trigger a new build.

@szepeviktor szepeviktor changed the base branch from master to add/caching-route-method February 12, 2020 17:42
@szepeviktor szepeviktor changed the base branch from add/caching-route-method to master February 12, 2020 17:42
@szepeviktor
Copy link
Contributor Author

szepeviktor commented Feb 12, 2020

@westonruter Could you please help me: cla signed, 2×changing base and CI still points to old repo

@szepeviktor
Copy link
Contributor Author

szepeviktor commented Feb 12, 2020

in #241
kép

@westonruter
Copy link
Collaborator

@szepeviktor ok, sorry, I meant a git-rebase. You can just merge the latest changes from GoogleChromeLabs:master instead.

@westonruter
Copy link
Collaborator

@szepeviktor I'm curious. How was this identified as an error? The WP_Theme::__get() method has:

			case 'version':
				return $this->get( 'Version' );
// ...
			default:
				return $this->offsetGet( $offset );

But then WP_Theme::offsetGet() has:

			case 'Version':
			case 'Status':
				return $this->get( $offset );

But I'm not clear on how this issue was detected in the first place. How is this detected with static analysis?

The benefit of this change is that the fallback call to WP_Theme::offsetGet() is avoided.

@westonruter westonruter added this to the 0.4 milestone Feb 12, 2020
@szepeviktor
Copy link
Contributor Author

szepeviktor commented Feb 12, 2020

@szepeviktor
Copy link
Contributor Author

szepeviktor commented Feb 12, 2020

...also magic __get() knows it as version

https://github.com/WordPress/WordPress/blob/cad04902d6a162ba8320f82a6c65c7eb58cf9759/wp-includes/class-wp-theme.php#L490

btw I think wp_get_theme( $stylesheet )->Version is not array item access but property access

@westonruter westonruter merged commit 9bd27f3 into GoogleChromeLabs:master Feb 12, 2020
@szepeviktor szepeviktor deleted the patch-1 branch July 28, 2023 01:59
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.

2 participants