Skip to:

Opened 8 years ago

Closed 2 months ago

#7018 closed enhancement (fixed)

Properly declare all properties on classes

Reported by: tw2113's profile tw2113 Owned by: espellcaste's profile espellcaste
Milestone: 14.0.0 Priority: normal
Severity: normal Version:
Component: Core Keywords: good-first-bug has-patch

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;

Attachments (2)

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

Download all attachments as: .zip

Change History (28)

#1 @tw2113
8 years ago

  • Description modified (diff)

#2 @DJPaul
8 years ago

  • Milestone changed from Awaiting Review to Future Release


#3 @tw2113
8 years ago







Need to decide on visibility of properties



#4 @tw2113
8 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.

7 years ago

Declare the types based on class instead of array.

#6 @espellcaste
5 years 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
5 years 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
5 years 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: ?

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

#9 @tw2113
5 years 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.

5 years ago

#11 @espellcaste
5 years ago

  • Milestone changed from 5.0.0 to Up Next

This ticket was mentioned in PR #54 on buddypress/buddypress by renatonascalves.

18 months ago

  • Keywords has-patch added; needs-patch needs-refresh removed

renatonascalves commented on PR #54:

18 months ago

@imath Initially, I was updating the types (i.e., bool => boolean) per case/situation, but I'll introduce a future pull request with a change in the project. In this way, we account for everything.

#14 @imath
18 months ago

  • Milestone changed from Up Next to 12.0.0

#15 @espellcaste
17 months ago

In 13399:

Properly declare missing properties to several classes.

See #7018

#16 @espellcaste
17 months ago

In 13400:

Set missing properties to the BuddyPress class.

See #7018

#17 @espellcaste
17 months ago

In 13404:

Properly declare missing properties to classes in the Members component.

See #7018

#18 @espellcaste
16 months ago

  • Owner espellcaste deleted

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

15 months ago

#20 @imath
14 months ago

In 13453:

Properly declare the BP_Nouveau_Customizer_Group_Nav $group prop

See #8820
See #7018

#21 @arafatjamil01
8 months ago

I have applied the patch from svn for all the Declarations. Everything looks good.

Last edited 8 months ago by arafatjamil01 (previous) (diff)

#22 @imath
6 months ago

  • Milestone changed from 12.0.0 to Up Next

There are still a lot of work about this ticket, let's carry on during next release cycle.

#23 @espellcaste
6 months ago

In 13662:

Properly declare the BP_Groups_Component $table_name prop.

See #7018

#24 @imath
6 months ago

  • Milestone changed from Up Next to 14.0.0

#25 @espellcaste
3 months ago

  • Owner set to espellcaste

I think we got all of them. But I'll review the code base before resolving this ticket.

#26 @espellcaste
2 months ago

  • Resolution set to fixed
  • Status changed from assigned to closed

Resolving ticket since I can't find instances to be update now.

Note: See TracTickets for help on using tickets.