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

Raise PHPStan level to 3 #4731

Closed
wants to merge 216 commits into from
Closed

Conversation

schlessera
Copy link
Collaborator

Summary

This PR does the following:

  • Raises level of PHPStan checks to 3
  • Fixes lots of type-hints
  • Changes some return values so they match their respective signature
  • Unsets a callback to avoid using a nullable type
  • Sets textContent to an empty string instea dof null to adhere to the DOM specification (converted bts anyway)

Addresses Issue #4377

Checklist

  • My pull request is addressing an open issue (please create one otherwise).
  • My code is tested and passes existing tests.
  • My code follows the Engineering Guidelines (updates are often made to the guidelines, check it out periodically).

amedina and others added 30 commits December 6, 2018 21:57
Merge 1.0 release branch into master
Merge 1.0 release branch into master for 1.0.1 release
* '1.0' of github.com:ampproject/amp-wp: (29 commits)
  Fix readme date and typo
  Update readme with late 1.0.2 changes
  Restore cursor:pointer for menu-scroll-down after #1777
  Move wp-i18n and wp-dom-ready into src directory
  Eliminate needless building of amp-validation-tooltips.js
  Exclude assets/src from build
  Bump version to 1.0.2 and add changelog entry
  Update welcome prompt to point to Getting Started section on amp-wp.org
  Restrict assets to only be enqueued on amp_validated_url edit post screen
  Remove erroneous call to wp() in AMP_Widget_Text::inject_video_max_width_style()
  Add _doing_it_wrong() call when is_amp_endpoint() is called before wp action
  Remove unnecessary condition around polyfilling wp-i18n and wp-dom-ready
  Remove WP-CLI dependency for build
  Remove obsolete languages directory
  Make get_validated_url_file_path() better compatible with register_theme_directory()
  Add WP-CLI i18n command as explicit Composer dev dependency
  Improve performance of make-pot command by excluding more paths
  Add missing translators comment in amp-block-validation.js
  Polyfill wp-i18n and wp-dom-ready for non-Gutenberg 4.9 installs
  Re-install WP dom-ready & i18n as devDependencies
  ...
* 'develop' of github.com:ampproject/amp-wp: (389 commits)
  Run npm audit fix
  Bump version to 1.1.0
  Remove obsolete checks in verify-version-consistency.php
  Replace 'our' with 'the'
  Add link to ecosystem page on AMP settings screen
  Fix compatibility with PWA v0.1 by checking if constant exists
  Bump to 1.1-RC1
  Only initialize noscript fallback attributes property if needed.
  Keep args handling in individual sanitizers rather than noscript fallback trait.
  Use more specific names for noscript fallback trait methods, to prevent future conflicts.
  Iterate over node attributes backwards and omit double loop.
  Introduce AMP_Noscript_Fallback trait to unify noscript functionality in sanitizers.
  Exclude build from phpcs; fix phpcs error in test
  Preemptively remove disallowed attributes on audio/video/iframe noscript fallbacks
  Fix logic for when to add_noscript_fallback
  Update version numbers to 1.1
  Prevent adding img to noscript if it lacks HTTPS protocol
  Allow preventing adding noscript fallbacks for amp-video/amp-img
  Include add_noscript_fallback arg for audio/iframe sanitizer to use for STAMP
  Fix logic for adding noscript; add tests for add_noscript_fallback
  ...
Cherry-picks and squashes #2140 into 1.1 branch

* Use sprintf for deprecated notice
* Use placeholder for PWA warning string
* Do not use numbered placeholder if there is only one
* Remove errant space
* Fix some more translator comments
* tag '1.1.1':
  Update release instructions
  Bump version to 1.1.1
  Prevent wp_targeted_link_rel() from corrupting JSON in amp_validation_error term_description
  Fix first_child_tag_name_oneof constraint to only apply if first child exists
  Fix some more i18n strings and placeholders
  Fix link to outdated wiki page for AMP analytics
  Bump 1.1 branch to 1.1.1-alpha
…layout

* Eliminate fetching image dimensions for layouts that don't require them
* Allow special 'auto' value for width for layout=fixed-height.
pierlon and others added 24 commits April 10, 2020 09:55
* Fix unit tests

* Add loading attribute depending on WP version
* Add interfaces for ARIA roles

* Use Role interface in existing transformers

* Add new a11y sanitizer

* Port MarkupComparison trait over to AmpProject\AmpWP

* Use MarkupComparison trait instead of copy-pasted methods

* Add tests for new a11y sanitizer

* Fix tab/space code style issue

* Actually include the new sanitizer in sanitization

* Add test for missing both attrs, and preserving tabindex

Co-authored-by: Weston Ruter <[email protected]>
…4579)

* Update tests after block-library/style.css changes in Gutenberg 7.9

* Use CSS selector present since 5.0
* Update Optimizer spec tests

* Stub request for scenario 'AmpRuntimeCss - always_inlines_v0css'

* Note why this change was made

Co-Authored-By: Alain Schlesser <[email protected]>

Co-authored-by: Alain Schlesser <[email protected]>
* Fix an issue with a width attribute in a mustache template

When there's a mustache template that
has an element like
<amp-img width="{{width}}">,
there was an issue with invalid width.
But this proabably doesn't need to be validated.

* Align => in arrays, in response to failed Travis build

* Add failing test for when element is not a direct descendant of template

* Account for nodes in nested elements inside mustache templates

* Reuse is_inside_mustache_template to decide whether to skip validating attributes

Fixes #4446

* Address PHPCS issue

Add 2 spaces, in response
to a Travis issue.

* Account for Mustache script templates in forms

* Add a helper method for whether a node has a layout attribute with {{ }} syntax

Similar to the one in the AMP validator,
as Weston pointed out.

* Skip processing style attributes in Mustache scripts

* Commit Weston's suggestion to use the same regex from the style sanitizer

Co-Authored-By: Weston Ruter <[email protected]>

* Use the Attribute constants in place of string literals

Like Attribute::LAYOUT in
place of 'layout'.

* Make use of constants in is_inside_mustache_template

* Also use Tag constants in is_inside_mustache_template

* Add more <amp-img> to the test 'inside_amp_mustache_template_sizes'

* Add failing test for differing behavior from Validator

* Add heights and sizes attributes to all amp-img in mustache_templates_with_variable_attrs

* Fix test by removing invalid layout attributes

* Remove duplicated item in getMustacheTagPlaceholders

* Revert new test in AMP_Layout_Sanitizer_Test

Co-authored-by: Weston Ruter <[email protected]>
* tag '1.5.3':
  Bump 1.5.3
  Bump version to 1.5.3-RC1
  Fix handling of Mustache templates (#4583)
  Stub request based on test scenario (#4588)
  Update tests after block-library/style.css changes in Gutenberg 7.9 (#4579)
  Restrict doing plugin upgrade routine when not in admin (#4538)
  Add new accessibility sanitizer (#4535)
  Fix unit tests (#4564)
  Add button into Site Health to reenable CSS transient caching (#4522)
  Restore unification of multi-page post content in Reader mode (#4547)
  Prevent styles from being removed when in Customizer preview with Standard mode (#4553)
  Omit Jetpack from being activated during PHPUnit test runs (#4474)
  Mock Facebook embed tests (#4474)
  Mock Imgur embed tests (#4474)
  Use title case for Paired Browsing link in edit post screen (#4540)
  Ensure that validation query vars persist through redirects (#4544)
  Add requirements to plugin file header (#4543)
  Force status code of validation responses to be 200 (#4533)
  Update optimizer test specs (#4527)
  Bump 1.5.3-alpha
@googlebot
Copy link

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

@googlebot googlebot added the cla: no Has not signed the Google CLA label May 15, 2020
@schlessera
Copy link
Collaborator Author

Something went awfully wrong with this PR, closing in favor of a fresh take in #4732

@schlessera schlessera closed this May 15, 2020
@schlessera schlessera deleted the 4377-raise-phpstan-level-to-3 branch May 15, 2020 16:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: no Has not signed the Google CLA
Projects
None yet
Development

Successfully merging this pull request may close these issues.