Postponed until 8.1.x is open for development.

Similar to #1856630: [Change notice] [META] Rename Views methods to core standards

Problem/Motivation

  • Currently, many Views property names use underscores rather than camelCase.

Beta phase evaluation

Reference: https://www.drupal.org/core/beta-changes
Issue category Task because it is not a bug or feature request. https://www.drupal.org/core/issue-category
Issue priority Normal because this is a meta, but each child would be minor per https://www.drupal.org/core/issue-priority since the change is cosmetic
Unfrozen changes This is not only any of the unfrozen changes.
Prioritized changes This is not a prioritized change for the beta phase.

Thus is postponed to 8.1.x. similar to #1518116-86: [meta] Make Core pass Coder Review

Proposed resolution

  • Do not change class properties that would make changes in .yml files.
  • Rename properties according to the new standard. "class properties should use lowerCamel naming" https://drupal.org/node/608152#naming
  • Convert property names exactly from underscore_separated to camelCase. So,
    foo_the_bar
    becomes
    fooTheBar.
  • Change public to protected (depending whether the variable is used outside of the class).
  • Replace var with public/protected, see previous point.
  • Update the docblock for the properties to agree with https://drupal.org/node/1354#var

Before replacing any property names, we must check that the new name is not already in use.

If we change a public property name of a class, we need to change all uses of that property in other files as part of the issue.

Instructions for creating issues

Until all the issues are created, these instructions are here so they are in a central location.

  1. Use this link to create a sub-issue for each of the below.
  2. Edit this issue and add a link to the new sub-issue in the appropriate "Issue" column in this issue summary using the syntax [#nid@].
  3. Only files that declare a class need to have issues created..
In Variables Issue
lib/Drupal/views/Plugin/views/field/FieldPluginBase.php public $original_value #2052591: In FieldPluginBase Rename Views properties to core standards
views/Controller/ViewAjaxController.php #2099451: In ViewAjaxController Rename Views properties to core standards Assigned to: leeotzu
views/DisplayBag.php #2099483: In DisplayBag Rename Views properties to core standards Assigned to: leeotzu
views/Plugin/Core/Entity/View.php protected $base_table, protected $base_field #2062123: In View Rename Views properties to core standards Assigned to: Garbar
views/Plugin/Derivative/ViewsBlock.php protected $base_plugin_id #2062157: In ViewsBlock Rename Views properties to core standards Assigned to: Garbar
views/Plugin/views/argument/ArgumentPluginBase.php var $name_table, var $name_field #2062177: In ArgumentPluginBase Rename Views properties to core standards
views/Plugin/views/argument/Date.php var $option_name #2062255: In argument/Date Rename Views properties to core standards
views/Plugin/views/field/MachineName.php var $value_options #2062131: In MachineName Class Rename Views properties to core standards
views/Plugin/views/filter/FilterPluginBase.php var $group_info, var $always_multiple, var $no_operator, var $always_required #2066451: In FilterPluginBase Rename Views properties to core standards
views/Plugin/views/filter/Equality.php var $always_multiple #2063897: In Equality class Rename Views properties to core standards Assigned to: jlindsey15
views/Plugin/views/filter/String.php var $always_multiple #2062153: In String Class Rename Views properties to core standards
views/Plugin/views/wizard/WizardPluginBase.php protected $base_table, protected $entity_type, protected $entity_info, protected $filter_defaults #2078593: In WizardPluginBase Rename Views properties to core standards
views/Plugin/views/row/EntityRow.php public $base_table, public $base_field #2078627: In EntityRow Rename Views properties to core standards Assigned to: mac_weber
views/ResultRow.php public $_entity, public $_relationship_entities #2078607: In ResultRow Rename Views properties to core standards
views/ViewExecutable.php public $build_info, and 23 more #2083381: In Class ViewExecutable Rename Views properties to core standards
views/Plugin/views/filter/BooleanOperator.php var $always_multiple, var $no_operator, var $accept_null #2183415: In BooleanOperator Rename Views properties to core standards
views/Plugin/views/query/Sql.php var $table_queue, var $group_operator, var $get_count_optimized, var $field_aliases, var $no_distinct #2183417: In Sql Rename Views properties to core standards
views/Plugin/views/style/StylePluginBase.php var $row_tokens, protected $rendered_fields #2183419: In StylePluginBase Rename Views properties to core standards
views/Plugin/views/filter/InOperator.php var $value_form_type, var $value_options #2183421: In InOperator Rename Views properties to core standards
views/Plugin/views/filter/ManyToOne.php var $value_form_type #2183423: In ManyToOne Rename Views properties to core standards
views/Plugin/views/filter/Numeric.php var $always_multiple #2183425: In Numeric Rename Views properties to core standards
views/Plugin/views/pager/PagerPluginBase.php var $current_page, var $total_items #2183427: PagerPluginBase properties to be renamed according to core standards
In Variables Issue
views/Ajax/DismissFormCommand.php none
views/Ajax/HighlightCommand.php none
views/Ajax/ReplaceTitleCommand.php none
views/Ajax/ScrollTopCommand.php none
views/Ajax/SetFormCommand.php none
views/Ajax/ShowButtonsCommand.php none
views/Ajax/TriggerPreviewCommand.php none
views/Ajax/ViewAjaxResponse.php none
views/Analyzer.php none
views/EventSubscriber/RouteSubscriber.php none
views/ManyToOneHelper.php none
views/Plugin/Block/ViewsBlock.php none
views/Plugin/Block/ViewsExposedFilterBlock.php none
views/Plugin/Derivative/DefaultWizardDeriver.php none
views/Plugin/Derivative/ViewsEntityRow.php none
views/Plugin/Derivative/ViewsExposedFilterBlock.php none
views/Plugin/Discovery/ViewsHandlerDiscovery.php none
views/Plugin/entity_reference/selection/ViewsSelection.php none
views/Plugin/views/access/AccessPluginBase.php none
views/Plugin/views/access/None.php none
views/Plugin/views/area/AreaPluginBase.php none
views/Plugin/views/area/Broken.php none
views/Plugin/views/area/Entity.php none
views/Plugin/views/area/Result.php none
views/Plugin/views/area/Text.php none
views/Plugin/views/area/TextCustom.php none
views/Plugin/views/area/Title.php none
views/Plugin/views/area/TokenizeAreaPluginBase.php none
views/Plugin/views/area/View.php none
views/Plugin/views/argument/Broken.php none
views/Plugin/views/argument/DayDate.php none
views/Plugin/views/argument/Formula.php none
views/Plugin/views/argument/FullDate.php none
views/Plugin/views/argument/GroupByNumeric.php none
views/Plugin/views/argument/ManyToOne.php none
views/Plugin/views/argument/MonthDate.php none
views/Plugin/views/argument/Null.php none
views/Plugin/views/argument/Numeric.php none
views/Plugin/views/argument/Standard.php none
views/Plugin/views/argument/String.php none
views/Plugin/views/argument/WeekDate.php none
views/Plugin/views/argument/YearDate.php none
views/Plugin/views/argument/YearMonthDate.php none
views/Plugin/views/argument_default/ArgumentDefaultPluginBase.php none
views/Plugin/views/argument_default/Fixed.php none
views/Plugin/views/argument_default/Raw.php none
views/Plugin/views/argument_validator/ArgumentValidatorPluginBase.php none
views/Plugin/views/argument_validator/None.php none
views/Plugin/views/argument_validator/Numeric.php none
views/Plugin/views/cache/CachePluginBase.php none
views/Plugin/views/cache/None.php none
views/Plugin/views/cache/Time.php none
views/Plugin/views/display/Attachment.php none
views/Plugin/views/display/DefaultDisplay.php none
views/Plugin/views/display/DisplayPluginBase.php none
views/Plugin/views/display/Embed.php none
views/Plugin/views/display/Feed.php none
views/Plugin/views/display/Page.php none
views/Plugin/views/display/PathPluginBase.php none
views/Plugin/views/display_extender/DefaultDisplayExtender.php none
views/Plugin/views/display_extender/DisplayExtenderPluginBase.php none
views/Plugin/views/exposed_form/Basic.php none
views/Plugin/views/exposed_form/ExposedFormPluginBase.php none
views/Plugin/views/exposed_form/InputRequired.php none
views/Plugin/views/field/Boolean.php none
views/Plugin/views/field/Broken.php none
views/Plugin/views/field/Counter.php none
views/Plugin/views/field/Custom.php none
views/Plugin/views/field/Date.php none
views/Plugin/views/field/Dropbutton.php none
views/Plugin/views/field/EntityLabel.php none
views/Plugin/views/field/FileSize.php none
views/Plugin/views/field/Links.php none
views/Plugin/views/field/Markup.php none
views/Plugin/views/field/Numeric.php none
views/Plugin/views/field/PrerenderList.php none
views/Plugin/views/field/Serialized.php none
views/Plugin/views/field/Standard.php none
views/Plugin/views/field/TimeInterval.php none
views/Plugin/views/field/Url.php none
views/Plugin/views/field/Xss.php none
views/Plugin/views/filter/BooleanOperatorString.php none
views/Plugin/views/filter/Broken.php none
views/Plugin/views/filter/Bundle.php none
views/Plugin/views/filter/Combine.php none
views/Plugin/views/filter/Date.php none
views/Plugin/views/filter/GroupByNumeric.php none
views/Plugin/views/filter/Standard.php none
views/Plugin/views/HandlerBase.php none
views/Plugin/views/join/JoinPluginBase.php (has also class: JoinComplex) none
views/Plugin/views/join/Standard.php none
views/Plugin/views/join/Subquery.php none
views/Plugin/views/pager/Full.php none
views/Plugin/views/pager/Mini.php none
views/Plugin/views/pager/None.php none
views/Plugin/views/pager/Some.php none
views/Plugin/views/pager/SqlBase.php none
views/Plugin/views/PluginBase.php none
views/Plugin/views/query/QueryPluginBase.php none
views/Plugin/views/relationship/Broken.php none
views/Plugin/views/relationship/GroupwiseMax.php none
views/Plugin/views/relationship/RelationshipPluginBase.php none
views/Plugin/views/relationship/Standard.php none
views/Plugin/views/row/Fields.php none
views/Plugin/views/row/RowPluginBase.php none
views/Plugin/views/row/RssFields.php none
views/Plugin/views/sort/Broken.php none
views/Plugin/views/sort/Date.php none
views/Plugin/views/sort/GroupByNumeric.php none
views/Plugin/views/sort/MenuHierarchy.php none
views/Plugin/views/sort/Random.php none
views/Plugin/views/sort/SortPluginBase.php none
views/Plugin/views/sort/Standard.php none
views/Plugin/views/style/DefaultStyle.php none
views/Plugin/views/style/DefaultSummary.php none
views/Plugin/views/style/Grid.php none
views/Plugin/views/style/HtmlList.php none
views/Plugin/views/style/Mapping.php none
views/Plugin/views/style/Rss.php none
views/Plugin/views/style/Table.php none
views/Plugin/views/style/UnformattedSummary.php none
views/Plugin/views/wizard/Standard.php none
views/Plugin/views/wizard/WizardException.php none
views/Plugin/ViewsHandlerManager.php none
views/Plugin/ViewsPluginManager.php none
views/Routing/ViewPageController.php none
views/ViewAccessController.php none
views/ViewExecutableFactory.php none
views/Views.php none
views/ViewsAccessCheck.php none
views/ViewsData.php none
views/ViewsDataHelper.php none
views/ViewStorageController.php none

Follow ups

  • ?

Comments

dawehner’s picture

dawehner
Primary language German
commented
Issue summary: View changes

updated

yesct’s picture

yesct commented
Issue summary: View changes

updating link for creating new issues

yesct’s picture

yesct commented
Issue summary: View changes

I'm not sure if it makes sense to list the properties in the table

yesct’s picture

yesct commented

1.
When renaming, should we
Update the docblock for the properties to agree with https://drupal.org/node/1354#var
?

Looking at a sample of issues from #1856630: [Change notice] [META] Rename Views methods to core standards, it looks like those did *not* add missing doc blocks to functions that were being renamed.

2.
Is there an easy way to list all the classes?

yesct’s picture

yesct commented
Issue summary: View changes

Updated issue summary.

yesct’s picture

yesct commented

Well, I made the list of files with:
grep -R "class " core/modules/views | grep " {" | grep -v "\/Tests\/" | grep -v "\/tests\/" | grep -v " if [(]" | sed 's/abstract //' | grep -v " \* " | grep -v " function " | grep -v " foreach " | cut -d":" -f1 | cut -c"31-" | awk '{print " <tr><td>" $1 "</td><td></td><td></td><td>[#@]</td></tr>"}'

Oops. I dont want to actually skip the line with * class.
Adding that file (JoinPluginBase) back in manually.
[edit: oh, that was core/modules/views/lib/Drupal/views/Plugin/views/join/JoinPluginBase.php: * class JoinComplex extends JoinPluginBase {
and JoinPluginBase was already listed. Added a note in the table that it has an additional class in it.]

yesct’s picture

yesct commented
Issue summary: View changes

added all the files to the table

yesct’s picture

yesct commented
Issue summary: View changes

added class JoinComplex

yesct’s picture

yesct commented
Issue summary: View changes

marked the ones that have no properties that need to be renamed and noted the var names in the ones that do

yesct’s picture

yesct commented

I checked all the files with a script and updated the table in the issue summary.
there are only a few that need issues, the ones that do not have any variables that need renaming I marked as "none" in the issue column.

used the previous command and made a file with the list of files in it
grep -R "class " core/modules/views | grep " {" | grep -v "\/Tests\/" | grep -v "\/tests\/" | grep -v " if [(]" | sed 's/abstract //' | grep -v " \* " | grep -v " function " | grep -v " foreach " | cut -d":" -f1 > ~/drupal-stuff/viewsclasses.all.txt

then did:

#!/bin/bash
  while IFS= read -r file
  do
    echo $file    grep -e "var \\$" -e "public \\$" -e "protected \\$" -e "private \\$" $file | grep -e "_"
  done < "/Users/ctheys/drupal-stuff/viewsclasses.all.txt"
yesct’s picture

yesct commented
Issue summary: View changes

had one the wrong column

danylevskyi’s picture

danylevskyi
Location Myrhorod / Ukraine
commented
Issue tags: +CodeSprintCIS
danylevskyi’s picture

danylevskyi
Location Myrhorod / Ukraine
commented
Issue summary: View changes

Updated issue summary.

danylevskyi’s picture

danylevskyi
Location Myrhorod / Ukraine
commented
Issue summary: View changes

Assigned issue.

danylevskyi’s picture

danylevskyi
Location Myrhorod / Ukraine
commented
Issue summary: View changes

Assigned issue.

Garbar’s picture

Garbar commented
Issue summary: View changes

Updated issue summary.

Garbar’s picture

Garbar commented
Issue summary: View changes

Updated issue summary.

Garbar’s picture

Garbar commented
Issue summary: View changes

Updated issue summary.

jlindsey15’s picture

jlindsey15 commented

I was doing the issue for Equality.php, where the relevant property is $always_multiple. I was curious so I did a grep for always_multiple, and I found it in Search.php and Name.php, which aren't listed here... Should they be or am I missing something?

jlindsey15’s picture

jlindsey15 commented
Issue summary: View changes

Added child issue.

jlindsey15’s picture

jlindsey15 commented
Issue summary: View changes

Added sub-issue

mac_weber’s picture

mac_weber commented
Issue summary: View changes

Added issue for WizardPluginBase

mac_weber’s picture

mac_weber commented
Issue summary: View changes

Added issue for ResultRow #2078607

mac_weber’s picture

mac_weber commented
Issue summary: View changes

Adds link to EntityRow #2078627

mac_weber’s picture

mac_weber commented

I just added instructions to not change class properties that would make changes in .yml files.

gistland’s picture

gistland commented

Wow. Terrific!!!

gistland’s picture

gistland commented
Issue summary: View changes

Adds instructions to not make changes in class properties that changes yml files

lokapujya’s picture

lokapujya
Primary language English
Location Boston
commented

I think you need to handle the dependencies of any function you rename. For example, other files may call the methods that you have changed.

dawehner’s picture

dawehner
Primary language German
commented

Just to be sure, I don't think a file based approach will work at all.

What you rather need is a variable name based approach, as you have to rename all its definitions and uses at once in order to not break it. Note: Views does not have covered every single line of code,
so renaming variables here and there is potentially problematic.

dawehner’s picture

dawehner
Primary language German
commented
Issue summary: View changes

Updated issue summary.

leeotzu’s picture

leeotzu commented
Issue summary: View changes

Updated issue summary.

leeotzu’s picture

leeotzu commented
Issue summary: View changes

Updated issue summary.

leeotzu’s picture

leeotzu commented
Issue summary: View changes

Updated issue summary.

lokapujya’s picture

lokapujya
Primary language English
Location Boston
commented
Issue summary: View changes
lokapujya’s picture

lokapujya
Primary language English
Location Boston
commented
Issue summary: View changes
lokapujya’s picture

lokapujya
Primary language English
Location Boston
commented
Issue summary: View changes
lokapujya’s picture

lokapujya
Primary language English
Location Boston
commented

Updated the issue summary to show that you are responsible for all properties in the class and updating any place that uses the public properties.

lokapujya’s picture

lokapujya
Primary language English
Location Boston
commented
Issue summary: View changes
lokapujya’s picture

lokapujya
Primary language English
Location Boston
commented
Issue summary: View changes
lokapujya’s picture

lokapujya
Primary language English
Location Boston
commented
Issue summary: View changes
lokapujya’s picture

lokapujya
Primary language English
Location Boston
commented
Issue summary: View changes
internetdevels’s picture

internetdevels commented
Issue summary: View changes

Data grouped from one table to two separate tables.

bohart’s picture

bohart
Location Lutsk, Ukraine
commented
Issue summary: View changes

All sub-issues was created and added to table.

yesct’s picture

yesct commented
Issue summary: View changes

See #2078593-15: In WizardPluginBase Rename Views properties to core standards. The children of this meta, and this meta might be postponed to 8.1.x being open for development according to https://www.drupal.org/contribute/core/beta-changes

yesct’s picture

yesct commented
Version: 8.0.x-dev » 8.1.x-dev
Issue summary: View changes
Status: Active » Postponed
Issue tags: -Novice

yeah, I'm more confident this is postponed. updated issue summary section more.
Thus removing the Novice tag per https://www.drupal.org/core-mentoring/novice-tasks

yesct’s picture

yesct commented
Issue summary: View changes

So.. I'm gonna postpone all the children, and set them to minor tasks in 8.1.x
[edit:]
I think it should be 8.1.x where possible to do in a non API BC breaking way.

For example: any property that is protected will not be API breaking to rename it.
Also, any property that has a getter/setter... we should consider it not API breaking to rename, cause contrib and core should be using the getter and setter anyway.

For properties that have no methods to access them when needed... that didn't get refactored to be good OOP in time, I guess those should be 9.0.x?

chi’s picture

chi commented
Status: Postponed » Active

8.1.x is already open for development.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.0-beta1 was released on March 2, 2016, which means new developments and disruptive changes should now be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.0-beta1 was released on August 3, 2016, which means new developments and disruptive changes should now be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

andypost’s picture

andypost
he/him
Primary language Russian
commented

Filed #3344578: In StylePluginBase Rename Views properties to core standards

quietone’s picture

quietone commented

This issue is making corrections to conform to coding standards. Changes of this type are now done by sniff, not individual file, see #2571965: [meta] Fix PHP coding standards in core, stage 1 for history of the change. I think all the child issues should be closed and credit moved to a new issue to Fix Drupal.NamingConventions.ValidVariableName.LowerCamelName.

quietone’s picture

quietone commented
Parent issue: » #3346468: [meta] Fix class properties violating Drupal.NamingConventions.ValidVariableName.LowerCamelName

There are more than just views files that the sniff reports with the problem. So, I am making this a child to the meta for fixing Drupal.NamingConventions.ValidVariableName.LowerCamelName.

quietone’s picture

quietone commented
Status: Active » Closed (duplicate)

After some more thought, I am going to close this issue dues to the reasons in #39 and transfer credit to the Meta for implementing the sniff, #3346468: [meta] Fix class properties violating Drupal.NamingConventions.ValidVariableName.LowerCamelName. I am choosing duplicate as that seems the best fit.

Now, what to do with the children. The child issue will have changes that are still valid and credit should be given for that work, according to policy. For now, I will postpone them and make a comment on the new meta about them. That will prevent people from working on them until we have agreement on how to deprecate these properties.