Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
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
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
tocamelCase
. 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.
- Use this link to create a sub-issue for each of the below.
- 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@]
. - 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 Assigned to: quietone |
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: In PagerPluginBase Rename Views properties 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
Comment #0.0
dawehnerupdated
Comment #0.1
YesCT CreditAttribution: YesCT commentedupdating link for creating new issues
Comment #0.2
YesCT CreditAttribution: YesCT commentedI'm not sure if it makes sense to list the properties in the table
Comment #1
YesCT CreditAttribution: YesCT commented1.
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?
Comment #1.0
YesCT CreditAttribution: YesCT commentedUpdated issue summary.
Comment #2
YesCT CreditAttribution: YesCT commentedWell, 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.]
Comment #2.0
YesCT CreditAttribution: YesCT commentedadded all the files to the table
Comment #2.1
YesCT CreditAttribution: YesCT commentedadded class JoinComplex
Comment #2.2
YesCT CreditAttribution: YesCT commentedmarked the ones that have no properties that need to be renamed and noted the var names in the ones that do
Comment #3
YesCT CreditAttribution: YesCT commentedI 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:
Comment #3.0
YesCT CreditAttribution: YesCT commentedhad one the wrong column
Comment #4
danylevskyiComment #4.0
danylevskyiUpdated issue summary.
Comment #4.1
danylevskyiAssigned issue.
Comment #4.2
danylevskyiAssigned issue.
Comment #4.3
Garbar CreditAttribution: Garbar commentedUpdated issue summary.
Comment #4.4
Garbar CreditAttribution: Garbar commentedUpdated issue summary.
Comment #4.5
Garbar CreditAttribution: Garbar commentedUpdated issue summary.
Comment #5
jlindsey15 CreditAttribution: jlindsey15 commentedI 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?
Comment #5.0
jlindsey15 CreditAttribution: jlindsey15 commentedAdded child issue.
Comment #5.1
jlindsey15 CreditAttribution: jlindsey15 commentedAdded sub-issue
Comment #5.2
Mac_Weber CreditAttribution: Mac_Weber commentedAdded issue for WizardPluginBase
Comment #5.3
Mac_Weber CreditAttribution: Mac_Weber commentedAdded issue for ResultRow #2078607
Comment #5.4
Mac_Weber CreditAttribution: Mac_Weber commentedAdds link to EntityRow #2078627
Comment #6
Mac_Weber CreditAttribution: Mac_Weber commentedI just added instructions to not change class properties that would make changes in
.yml
files.Comment #7
gistland CreditAttribution: gistland commentedWow. Terrific!!!
Comment #7.0
gistland CreditAttribution: gistland commentedAdds instructions to not make changes in class properties that changes yml files
Comment #8
lokapujyaI think you need to handle the dependencies of any function you rename. For example, other files may call the methods that you have changed.
Comment #9
dawehnerJust 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.
Comment #9.0
dawehnerUpdated issue summary.
Comment #9.1
leeotzu CreditAttribution: leeotzu commentedUpdated issue summary.
Comment #9.2
leeotzu CreditAttribution: leeotzu commentedUpdated issue summary.
Comment #9.3
leeotzu CreditAttribution: leeotzu commentedUpdated issue summary.
Comment #10
lokapujyaComment #11
lokapujyaComment #12
lokapujyaComment #13
lokapujyaUpdated the issue summary to show that you are responsible for all properties in the class and updating any place that uses the public properties.
Comment #14
lokapujyaComment #15
lokapujyaComment #16
lokapujyaComment #17
lokapujyaComment #18
InternetDevels CreditAttribution: InternetDevels commentedData grouped from one table to two separate tables.
Comment #19
bohartAll sub-issues was created and added to table.
Comment #20
YesCT CreditAttribution: YesCT commentedSee #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
Comment #21
YesCT CreditAttribution: YesCT commentedyeah, 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
Comment #22
YesCT CreditAttribution: YesCT commentedSo.. 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?
Comment #23
Chi CreditAttribution: Chi commented8.1.x is already open for development.
Comment #38
andypostFiled #3344578: In StylePluginBase Rename Views properties to core standards
Comment #39
quietone CreditAttribution: quietone at PreviousNext commentedThis 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 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.
Comment #40
quietone CreditAttribution: quietone at PreviousNext commentedThere 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.
Comment #41
quietone CreditAttribution: quietone at PreviousNext commentedAfter 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 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.