Skip to:
Content

BuddyPress.org

Opened 3 years ago

Last modified 2 months ago

#7018 assigned enhancement

Properly declare all properties on classes

Reported by: tw2113 Owned by: espellcaste
Milestone: Up Next Priority: normal
Severity: normal Version:
Component: Core Keywords: needs-patch needs-refresh good-first-bug
Cc:

Description (last modified by tw2113)

We have a number of classes available, whether internal or public for users, and that's wonderful. However, we have not always properly declared the properties that we set in the __construct() methods or elsewhere in the class. We should work to change that fact.

Example from BP_Groups_Group:

/**
 * ID of the group.
 *
 * @since 1.6.0
 * @var int
 */
public $id;
public function __construct( $id = null, $args = array() ) {
	$this->args = wp_parse_args( $args, array(
		'populate_extras' => false,
	) );

	if ( !empty( $id ) ) {
		$this->id = $id;
		$this->populate();
	}
}

Attachments (2)

7018-declare-component-properties.diff (1.5 KB) - added by tw2113 2 years ago.
7018-declare-component-properties.2.diff (1.6 KB) - added by tw2113 2 years ago.
Declare the types based on class instead of array.

Download all attachments as: .zip

Change History (13)

#1 @tw2113
3 years ago

  • Description modified (diff)

#2 @DJPaul
3 years ago

  • Milestone changed from Awaiting Review to Future Release

Indeed

#3 @tw2113
3 years ago

CHECK FOR MULTIPLE SPOTS

class-bp-activity-activity.php

{$bp->activity->table_name}
buddypress()->activity->table_name_meta
$bp->profile->table_name_data

class-bp-activity-component.php

buddypress()->plugin_dir
$bp->table_prefix
$bp->pages->activity->slug
buddypress()->my_account_menu_id
$bp->bp_options_title
$bp->bp_options_avatar

class-bp-activity-feed.php

Need to decide on visibility of properties

class-bp-activity-list-table

buddypress()->profile->name

#4 @tw2113
3 years ago

Some appear to be a part of BuddyPress::setup_globals(); which has a nice long note about protecting things and keeping part of the galaxy at peace.

@tw2113
2 years ago

Declare the types based on class instead of array.

#6 @espellcaste
8 months ago

  • Keywords needs-patch needs-refresh added
  • Milestone changed from Awaiting Contributions to 5.0.0

@tw2113 This seems to be an interesting problem we could solve. Can you leave an update what's left on this one?

#7 @tw2113
8 months ago

Honestly probably most of it. I doubt I ever committed anything officially, instead just relied on the diff files above for extra review.

#8 @espellcaste
8 months ago

  • Keywords good-first-bug added
  • Owner set to espellcaste
  • Status changed from new to assigned

@tw2113 Can I ask how you generated this list here: https://gist.github.com/tw2113/b41f25858d0434e794284edd8db7c954 ?

And thank you for your feedback, I'll take from here. :)

#9 @tw2113
8 months ago

I most likely did a grep/ack/ag code search on the trunk branch and extracted out the results.

This ticket was mentioned in Slack in #buddypress by espellcaste. View the logs.


2 months ago

#11 @espellcaste
2 months ago

  • Milestone changed from 5.0.0 to Up Next
Note: See TracTickets for help on using tickets.