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

Added autoloader to reduce complexity; fix phpcs issues #828

Merged
merged 49 commits into from
Dec 11, 2017
Merged
Show file tree
Hide file tree
Changes from 48 commits
Commits
Show all changes
49 commits
Select commit Hold shift + click to select a range
8346524
Added classmap autoloader, removed all require_once(), deprecated amp…
mikeschinkel Dec 8, 2017
5c1fa20
Remove duplicated array index.
mikeschinkel Dec 8, 2017
33c0ebf
Initialize array that is used by may not have been initialized.
mikeschinkel Dec 8, 2017
08bac6b
Removed unused variables.
mikeschinkel Dec 8, 2017
e5c1a8c
Removed 2nd parameter to remove_shortcode() because remove_shortcode(…
mikeschinkel Dec 8, 2017
8b267ef
Added some PHPDoc for methods.
mikeschinkel Dec 8, 2017
8b34e24
Added PHPDoc so PhpStorm will not flag as an error.
mikeschinkel Dec 8, 2017
b54243b
Add dev-lib
westonruter Nov 5, 2017
38cccab
Remove partial PHPDoc header.
mikeschinkel Dec 9, 2017
f13c385
Restructure autoloader to use static methods.
mikeschinkel Dec 9, 2017
03c126b
Document the sanitizers
mikeschinkel Dec 9, 2017
fdd1c74
Documented AMP DOM Utils.
mikeschinkel Dec 9, 2017
48c492e
Updating reference to dev-lib
mikeschinkel Dec 9, 2017
1a0b53c
Documented PHPDoc header for amp_get_permalink()
mikeschinkel Dec 9, 2017
fb10624
Renamed _build_placeholder() back to build_placeholder() since no whe…
mikeschinkel Dec 9, 2017
19b6f04
Remove errant tick mark.
mikeschinkel Dec 9, 2017
b1ee243
Revert string concatenation style change.
mikeschinkel Dec 9, 2017
010dad4
Revert regex syntax style change.
mikeschinkel Dec 9, 2017
8fad986
Add class AMP_Rule_Spec in its own file.
mikeschinkel Dec 9, 2017
241e866
Merge branch 'develop' of https://github.com/Automattic/amp-wp into n…
westonruter Dec 10, 2017
38992ed
Fix autoloader file refs and PHP 5.2 compat
westonruter Dec 10, 2017
5fcbc2b
Fix phpcs
westonruter Dec 10, 2017
2105af0
Stop <amg-audio> sanitizer from including a contained "\n" (and thus …
mikeschinkel Dec 11, 2017
f85c366
Revert the unfortunate change that broke placeholders in iframes.
mikeschinkel Dec 11, 2017
564a9d9
Ensure that no errant whitespace text nodes are added to output. Thi…
mikeschinkel Dec 11, 2017
f90c16d
Exclude includes/lib from linting; temporarily disable phpcs
westonruter Dec 11, 2017
b62132a
Change self::class to hardcode the class to support earlier versions …
mikeschinkel Dec 11, 2017
5417aeb
Fix improperly added filepath for the FastImage class in the autoloader.
mikeschinkel Dec 11, 2017
19658c9
Merge branch 'develop' of github.com:newclarity/amp-wp into develop
mikeschinkel Dec 11, 2017
a66c992
Merge branch 'develop' of github.com:newclarity/amp-wp into develop
mikeschinkel Dec 11, 2017
23bf64f
Fixed autoloader for PHP 5.2, e.g. __DIR__ ==> dirname( __FILE__ )
mikeschinkel Dec 11, 2017
bfa54bc
PHP5.2: Remove '\' from on front of ReflectionClass() because PHP 5.2…
mikeschinkel Dec 11, 2017
129bda7
Added PHPDoc explaining relative filepaths not having leading slash o…
mikeschinkel Dec 11, 2017
b1feee0
Added logic in AMP_DOM_Utils::get_content_from_dom() to replace <br><…
mikeschinkel Dec 11, 2017
00bea9b
Another attempt to fix unit test.
mikeschinkel Dec 11, 2017
b74ee8f
Another attempt to fix unit test, take two.
mikeschinkel Dec 11, 2017
6000b50
Ensure that content if retrieved via get_content_from_dom() and not v…
mikeschinkel Dec 11, 2017
6e0c13b
Added a composer.json.
mikeschinkel Dec 11, 2017
bd97046
Handle "no body" in AMP_DOM_Utils::get_content_from_dom()
mikeschinkel Dec 11, 2017
215a27b
Split get_content_from_dom() in to by adding get_content_from_dom_nod…
mikeschinkel Dec 11, 2017
85a46ba
Use the new AMP_DOM_Utils::get_content_from_dom_node() to make failin…
mikeschinkel Dec 11, 2017
de7f617
Remove "repositories" from composer.json because it is not merged in …
mikeschinkel Dec 11, 2017
5d1157f
Had to remove require: php>=5.2 because it appears to be breaking the…
mikeschinkel Dec 11, 2017
42c0600
Ugh. Have to go with minimal composer.json because it is breaking the…
mikeschinkel Dec 11, 2017
47edc2f
Need a commit to trigger a rebuild given that one of them timed out...
mikeschinkel Dec 11, 2017
3e15941
Merge branch 'develop' of github.com:Automattic/amp-wp into develop
ThierryA Dec 11, 2017
f5b2d2f
Include test/stubs in autoloader; remove unused absolute path support…
westonruter Dec 11, 2017
8b416f9
Fix phpcs issues in modified files
westonruter Dec 11, 2017
575ac66
Fix phpcs issues in AMP_Post_Template
westonruter Dec 11, 2017
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .dev-lib
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
SYNC_README_MD=0
SYNC_README_MD=0
PATH_EXCLUDES_PATTERN=includes/lib/*
20 changes: 4 additions & 16 deletions amp.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,12 @@
define( 'AMP__DIR__', dirname( __FILE__ ) );
define( 'AMP__VERSION', '0.6.0-alpha' );

require_once AMP__DIR__ . '/includes/class-amp-autoloader.php';
AMP_Autoloader::register();

require_once AMP__DIR__ . '/back-compat/back-compat.php';
require_once AMP__DIR__ . '/includes/amp-helper-functions.php';
require_once AMP__DIR__ . '/includes/class-amp-post-type-support.php';
require_once AMP__DIR__ . '/includes/admin/functions.php';
require_once AMP__DIR__ . '/includes/admin/class-amp-customizer.php';
require_once AMP__DIR__ . '/includes/admin/class-amp-post-meta-box.php';
require_once AMP__DIR__ . '/includes/settings/class-amp-customizer-settings.php';
require_once AMP__DIR__ . '/includes/settings/class-amp-customizer-design-settings.php';
require_once AMP__DIR__ . '/includes/actions/class-amp-frontend-actions.php';
require_once AMP__DIR__ . '/includes/actions/class-amp-paired-post-actions.php';

register_activation_hook( __FILE__, 'amp_activate' );
function amp_activate() {
Expand Down Expand Up @@ -150,7 +146,7 @@ function amp_maybe_add_actions() {
}

function amp_load_classes() {
require_once( AMP__DIR__ . '/includes/class-amp-post-template.php' ); // this loads everything else
_deprecated_function( __FUNCTION__, '0.6.0' );
}

function amp_add_frontend_actions() {
Expand Down Expand Up @@ -196,8 +192,6 @@ function amp_render_post( $post ) {
}
$post_id = $post->ID;

amp_load_classes();

/**
* Fires before rendering a post in AMP.
*
Expand Down Expand Up @@ -246,9 +240,3 @@ function amp_redirect_old_slug_to_new_url( $link ) {

return $link;
}

// Unconditionally load code required when running unit tests.
if ( function_exists( 'tests_add_filter' ) ) {
amp_load_classes();
require_once dirname( __FILE__ ) . '/tests/stubs.php';
}
7 changes: 7 additions & 0 deletions composer.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"name": "automattic/amp-wp",
"description": "WordPress plugin for adding AMP support.",
"homepage": "https://github.com/Automattic/amp-wp",
"type": "wordpress-plugin",
"license": "GPL-2.0"
}
2 changes: 1 addition & 1 deletion dev-lib
23 changes: 18 additions & 5 deletions includes/actions/class-amp-frontend-actions.php
Original file line number Diff line number Diff line change
@@ -1,14 +1,27 @@
<?php
// Callbacks for adding AMP-related things to the main theme

require_once( AMP__DIR__ . '/includes/actions/class-amp-actions.php' );
/**
* Class AMP_Frontend_Actions
*
* @package AMP
*/

/**
* Class AMP_Frontend_Actions
*
* Callbacks for adding AMP-related things to the main theme
*/
class AMP_Frontend_Actions {


/**
* Register hooks.
*/
public static function register_hooks() {
add_action( 'wp_head', 'AMP_Frontend_Actions::add_canonical' );
}


/**
* Add canonical link.
*/
public static function add_canonical() {
if ( false === apply_filters( 'add_canonical_link', true ) ) {
return;
Expand Down
96 changes: 77 additions & 19 deletions includes/actions/class-amp-paired-post-actions.php
Original file line number Diff line number Diff line change
@@ -1,9 +1,18 @@
<?php
/**
* Class AMP_Paired_Post_Actions
*
* @package AMP
*/

require_once( AMP__DIR__ . '/includes/actions/class-amp-actions.php' );

/**
* Class AMP_Paired_Post_Actions
*/
class AMP_Paired_Post_Actions extends AMP_Actions {


/**
* Register hooks.
*/
public static function register_hooks() {
add_action( 'amp_post_template_head', 'AMP_Paired_Post_Actions::add_generator_metadata' );
add_action( 'amp_post_template_head', 'AMP_Paired_Post_Actions::add_title' );
Expand All @@ -16,42 +25,75 @@ public static function register_hooks() {
add_action( 'amp_post_template_data', 'AMP_Paired_Post_Actions::add_analytics_scripts' );
add_action( 'amp_post_template_footer', 'AMP_Paired_Post_Actions::add_analytics_data' );
}


/**
* Add title.
*
* @param AMP_Post_Template $amp_template template.
*/
public static function add_title( $amp_template ) {
?>
<title><?php echo esc_html( $amp_template->get( 'document_title' ) ); ?></title>
<?php
}

/**
* Add canonical link.
*
* @param AMP_Post_Template $amp_template Template.
*/
public static function add_canonical_link( $amp_template ) {
?>
<link rel="canonical" href="<?php echo esc_url( $amp_template->get( 'canonical_url' ) ); ?>" />
<?php
}

/**
* Print scripts.
*
* @param AMP_Post_Template $amp_template Template.
*/
public static function add_scripts( $amp_template ) {
$scripts = $amp_template->get( 'amp_component_scripts', array() );
foreach ( $scripts as $element => $script ) :
$custom_type = ($element == 'amp-mustache') ? 'template' : 'element'; ?>
$custom_type = ( 'amp-mustache' === $element ) ? 'template' : 'element';
?>
<script custom-<?php echo esc_attr( $custom_type ); ?>="<?php echo esc_attr( $element ); ?>" src="<?php echo esc_url( $script ); ?>" async></script>
<?php endforeach; ?>
<script src="<?php echo esc_url( $amp_template->get( 'amp_runtime_script' ) ); ?>" async></script>
<?php
}

/**
* Print fonts.
*
* @param AMP_Post_Template $amp_template Template.
*/
public static function add_fonts( $amp_template ) {
$font_urls = $amp_template->get( 'font_urls', array() );
foreach ( $font_urls as $slug => $url ) : ?>
?>
<?php foreach ( $font_urls as $slug => $url ) : ?>
<link rel="stylesheet" href="<?php echo esc_url( $url ); ?>">
<?php endforeach;
<?php endforeach; ?>
<?php
}

/**
* Print boilerplate CSS.
*
* @param AMP_Post_Template $amp_template Template.
*/
public static function add_boilerplate_css( $amp_template ) {
?>
<style amp-boilerplate>body{-webkit-animation:-amp-start 8s steps(1,end) 0s 1 normal both;-moz-animation:-amp-start 8s steps(1,end) 0s 1 normal both;-ms-animation:-amp-start 8s steps(1,end) 0s 1 normal both;animation:-amp-start 8s steps(1,end) 0s 1 normal both}@-webkit-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@-moz-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@-ms-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@-o-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}</style><noscript><style amp-boilerplate>body{-webkit-animation:none;-moz-animation:none;-ms-animation:none;animation:none}</style></noscript>
<?php
}

/**
* Print Schema.org metadata.
*
* @param AMP_Post_Template $amp_template Template.
*/
public static function add_schemaorg_metadata( $amp_template ) {
$metadata = $amp_template->get( 'metadata' );
if ( empty( $metadata ) ) {
Expand All @@ -62,54 +104,70 @@ public static function add_schemaorg_metadata( $amp_template ) {
<?php
}

/**
* Print styles.
*
* @param AMP_Post_Template $amp_template Template.
*/
public static function add_styles( $amp_template ) {
$styles = $amp_template->get( 'post_amp_styles' );
if ( ! empty( $styles ) ) {
echo '/* Inline styles */' . PHP_EOL;
echo '/* Inline styles */' . PHP_EOL; // WPCS: XSS OK.
foreach ( $styles as $selector => $declarations ) {
$declarations = implode( ';', $declarations ) . ';';
printf( '%1$s{%2$s}', $selector, $declarations );
printf( '%1$s{%2$s}', $selector, $declarations ); // WPCS: XSS OK.
}
}
}


/**
* Add analytics scripts.
*
* @param array $data Data.
* @return array Data.
*/
public static function add_analytics_scripts( $data ) {
if ( ! empty( $data['amp_analytics'] ) ) {
$data['amp_component_scripts']['amp-analytics'] = 'https://cdn.ampproject.org/v0/amp-analytics-0.1.js';
}
return $data;
}

public static function add_analytics_data( $amp_template ) {

/**
* Print analytics data.
*
* @param AMP_Post_Template $amp_template Template.
*/
public static function add_analytics_data( $amp_template ) {
$analytics_entries = $amp_template->get( 'amp_analytics' );
if ( empty( $analytics_entries ) ) {
return;
}

foreach ( $analytics_entries as $id => $analytics_entry ) {
if ( ! isset( $analytics_entry['type'], $analytics_entry['attributes'], $analytics_entry['config_data'] ) ) {
/* translators: %1$s is analytics entry ID, %2$s is actual entry keys. */
_doing_it_wrong( __FUNCTION__, sprintf( esc_html__( 'Analytics entry for %1$s is missing one of the following keys: `type`, `attributes`, or `config_data` (array keys: %2$s)', 'amp' ), esc_html( $id ), esc_html( implode( ', ', array_keys( $analytics_entry ) ) ) ), '0.3.2' );
continue;
}

$script_element = AMP_HTML_Utils::build_tag( 'script', array(
'type' => 'application/json',
), wp_json_encode( $analytics_entry['config_data'] ) );

$amp_analytics_attr = array_merge( array(
'id' => $id,
'type' => $analytics_entry['type'],
), $analytics_entry['attributes'] );
echo AMP_HTML_Utils::build_tag( 'amp-analytics', $amp_analytics_attr, $script_element );

echo AMP_HTML_Utils::build_tag( 'amp-analytics', $amp_analytics_attr, $script_element ); // WPCS: XSS OK.
}
}

/**
* Add AMP generator metadata.
* Print AMP generator metadata.
*
* @param object $amp_template AMP_Post_Template object.
* @param AMP_Post_Template $amp_template AMP_Post_Template object.
* @since 0.6
*/
public static function add_generator_metadata( $amp_template ) {
Expand Down
10 changes: 6 additions & 4 deletions includes/admin/functions.php
Original file line number Diff line number Diff line change
@@ -1,9 +1,6 @@
<?php
// Callbacks for adding AMP-related things to the admin.

require_once AMP__DIR__ . '/includes/options/class-amp-options-menu.php';
require_once AMP__DIR__ . '/includes/options/views/class-amp-options-manager.php';

define( 'AMP_CUSTOMIZER_QUERY_VAR', 'customize_amp' );

/**
Expand Down Expand Up @@ -31,6 +28,11 @@ function amp_maybe_init_customizer() {
add_action( 'admin_menu', 'amp_add_customizer_link' );
}

/**
* Get permalink for the first AMP-eligible post.
*
* @return string|null
*/
function amp_admin_get_preview_permalink() {
/**
* Filter the post type to retrieve the latest for use in the AMP template customizer.
Expand All @@ -40,7 +42,7 @@ function amp_admin_get_preview_permalink() {
$post_type = (string) apply_filters( 'amp_customizer_post_type', 'post' );

if ( ! post_type_supports( $post_type, 'amp' ) ) {
return;
return null;
}

$post_ids = get_posts( array(
Expand Down
18 changes: 16 additions & 2 deletions includes/amp-helper-functions.php
Original file line number Diff line number Diff line change
@@ -1,11 +1,17 @@
<?php
/**
* AMP Helper Functions
*
* @package AMP
*/

/**
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing function description, param description, and return description. We've also been adding @since tags.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your requests to document this caused me to go down the rabbit hole for the past (far too many) hours. Every time I would document something it would cause PhpStorm to flag something else as being "off", so I ended up documented all of the sanitizers.

Note I found a few places that would throw errors, e.g. DOMNode $nodes using methods that are only on DOMElement so I added a variable of logic to ensure errors are not thrown during the normal course of usage when WP_DEBUG is set to true. One such example is I added the following in several places:

if ( ! $node instanceof DOMElement ) {
    return;
}

I hope that was okay...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The node type checks are probably a good idea, as I can see that clearly some of the situations would result in text nodes being used as elements. It looks like some of the instances you added would never result in a non-element being iterated (e.g. $this->dom->getElementsByTagName( self::$tag );) but it doesn't hurt.

* Get AMP permalink.
* Retrieves the full AMP-specific permalink for the given post ID.
*
* @since 0.1
*
* @param int $post_id Post ID.
*
* @return string AMP permalink.
*/
function amp_get_permalink( $post_id ) {
Expand All @@ -27,7 +33,7 @@ function amp_get_permalink( $post_id ) {
}

$parsed_url = wp_parse_url( get_permalink( $post_id ) );
$structure = get_option( 'permalink_structure' );
$structure = get_option( 'permalink_structure' );
if ( empty( $structure ) || ! empty( $parsed_url['query'] ) || is_post_type_hierarchical( get_post_type( $post_id ) ) ) {
$amp_url = add_query_arg( AMP_QUERY_VAR, '', get_permalink( $post_id ) );
} else {
Expand Down Expand Up @@ -64,6 +70,8 @@ function post_supports_amp( $post ) {
* Are we currently on an AMP URL?
*
* Note: will always return `false` if called before the `parse_query` hook.
*
* @return bool Whether it is the AMP endpoint.
*/
function is_amp_endpoint() {
if ( 0 === did_action( 'parse_query' ) ) {
Expand All @@ -73,6 +81,12 @@ function is_amp_endpoint() {
return false !== get_query_var( AMP_QUERY_VAR, false );
}

/**
* Get AMP asset URL.
*
* @param string $file Relative path to file in assets directory.
* @return string URL.
*/
function amp_get_asset_url( $file ) {
return plugins_url( sprintf( 'assets/%s', $file ), AMP__FILE__ );
}
Loading