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

[META] Finalizing PHPCS 2.x support #143

Open
5 of 6 tasks
mbabker opened this issue Jan 7, 2017 · 112 comments
Open
5 of 6 tasks

[META] Finalizing PHPCS 2.x support #143

mbabker opened this issue Jan 7, 2017 · 112 comments

Comments

@mbabker
Copy link
Contributor

mbabker commented Jan 7, 2017

Extracted from #109 (comment)

  • In the ruleset.xml there are some pear rules with adjusted messages that we either need to accept messages reporting spaces or will need to create custom copies to properly report the number of tabs.

  • Fix ControlStructuresBrackets sniff issues with the unitTests.

  • Testing against the CMS needs a custom ruleset as a custom standard to mute some errors. The readme has some explanations for adjustments. An example xml file should be included. I also can provide an xml example (code in previous comment way back) and another xml example for projects that have php5.4+ requirements like the Joomla-stats repo (xml code in previous comment just above)

  • Review what our custom sniffs are based on for changes in PHPCS 2.7.x that need to be adopted into our custom sniffs.

  • review for consistency with the old ruleset warnings/errors from phpcs 1.5.x (after the automatic fixers and manual fixes the 1.5.x ruleset should not throw any warnings/errors due to the code style phpcs 2.7.x version being applied. (I've been occasionally running against the CMS admin components for this check, I've only completed PR's for 3 or 4 components)

  • Merge coding standards manual to phpcs-2 branch

@photodude
Copy link
Contributor

photodude commented Jan 7, 2017

@photodude
Copy link
Contributor

photodude commented Jan 8, 2017

@mbabker where do you think the example xml files for selectively applying rules should live in the repo?

Added in ExampleRulesets via #152

@photodude
Copy link
Contributor

photodude commented Jan 9, 2017

@photodude
Copy link
Contributor

photodude commented Feb 11, 2017

Just notice this new issue with InstantiateNewClasses fixer as shown in the InstantiateNewClassUnitTest
If we instantiate a New Class as $k = new self(); we end up with $k = new ;
See https://travis-ci.org/photodude/coding-standards/jobs/200657756

InstantiateNewClass issue has been fixed, Thanks @wilsonge

we also have a bunch of Missing @package tag in file comment (Joomla.Commenting.FileComment.MissingPackageTag) issues now being reported as of phpcs 2.7.1+
fixed in #150

@photodude
Copy link
Contributor

photodude commented Feb 13, 2017

As best I can tell the ControlStructuresBrackets sniff issues are related to the section that deals with bracket indenting. https://github.com/joomla/coding-standards/blob/phpcs-2/Joomla/Sniffs/ControlStructures/ControlStructuresBracketsSniff.php#L148-L192

~~Likely in the str_repeat() with tabs~~~

ControlStructuresBrackets sniff issues have been fixed, Thanks @wilsonge

@photodude
Copy link
Contributor

photodude commented Feb 14, 2017

@wilsonge Are you still up for helping to work on these issues so we can move forward with the Alpha release?

This is the Branch I'm working with to try and create a PR to solve these issues.
https://github.com/photodude/coding-standards/tree/patch-2

@photodude
Copy link
Contributor

@rdeutz If your interested, this is the current list of Items to address with the PHPCS2 branch for a public alpha release of the updated standard with autofixers.

@photodude
Copy link
Contributor

photodude commented Feb 14, 2017

@nibra, @810, @vess if you are interested in continuing to follow the path to a public alpha release for the PHPCS2 branch, or contributing to fixing any of the issues in the PHPCS2 version of the standards prior to the alpha release this is issue to follow.

@wilsonge
Copy link
Contributor

I very much am!

@wilsonge
Copy link
Contributor

wilsonge commented Feb 14, 2017

I've done fixes for the InstantiateNewClasses and the ControlStructuresBrackets for the patch-5 (i made them against patch-2 like you asked - but locally been testing against patch-5 given that was where your unit test was running against that you linked to) branch and then the package tag warning looks correct to me

@photodude
Copy link
Contributor

photodude commented Feb 15, 2017

@photodude
Copy link
Contributor

photodude commented Feb 16, 2017

added a number of fixes see

@photodude
Copy link
Contributor

photodude commented Feb 16, 2017

Still need to

after that, I think the other review items can happen with the public alpha

@photodude
Copy link
Contributor

@mbabker @wilsonge Once we merge PRs 150-153 we just need to add composer instructions and we are set for the Alpha release. All other items are changes to the existing ruleset or are review items that can be handled with the alpha release.

@wilsonge
Copy link
Contributor

I've merged #150, #151 and #152. I want to have a quick look over #153 later today to convince myself that it is the best solution :)

@photodude
Copy link
Contributor

Thanks @wilsonge.

@photodude
Copy link
Contributor

I've added some composer install information (based on the framework composer instructions)

@photodude
Copy link
Contributor

With the merge of #153 we can check off the first item about dealing with the adjusted messages.
That leaves the review items and the proposed change to adopt the PSR-2 multiline class params.

How do we want to proceed with those items and making the official Alpha release?

@mbabker
Copy link
Contributor Author

mbabker commented Feb 17, 2017

Added a task on merging the manual to the right branch too since that's no longer in a self-contained branch (more of a reminder than anything since the dev site will read the manual from the master branch).

@photodude
Copy link
Contributor

I think there are also some changes to the IDE section in the Master branch that will need to be merged into the into phpcs-2 as well.

@wilsonge
Copy link
Contributor

So with merging #153 where does that leave us here?

@photodude
Copy link
Contributor

photodude commented Feb 18, 2017

The blog post needs to be finalized, @wilsonge have you reviewed the draft of that?

Dev Blog post

  • Finalize the Dev blog post about the public alpha release
  • Publish the Dev blog post about the public alpha release
    • Scheduled for publication on 2017/03/01

Create the release file(s)

  • Create release file(s) and packagist item(s)

Travis

As for other tasks, all of the following I feel can be addressed during the public alpha

Review

  • Review what our custom sniffs are based on for changes in PHPCS 2.7.x that need to be adopted into our custom sniffs.
  • Review for consistency with the old ruleset warnings/errors from phpcs 1.5.x after the automatic fixers and manual fixes have been applied

Updates

Changes

@wilsonge
Copy link
Contributor

Blog post looked good to me :)

@photodude
Copy link
Contributor

@mbabker Are we good now to release the public alpha?

@photodude
Copy link
Contributor

photodude commented Feb 19, 2017

Custom Sniffs Review

  • Classes/InstantiateNewClassesSniff.php
    • Based on (this seems to be a fully custom sniff)
    • Reason for Custom sniff
      • Instanciating new class without parameters does not require brackets.
    • reviewed on 2017/02/19
  • Commenting/ClassCommentSniff.php
    • Based on PEAR.Commenting.ClassComment Sniff
    • Reason for Custom sniff
      • Tags are in a different order and have different requierments,
    • reviewed on 2017/02/19
  • Commenting/FileCommentSniff.php
  • Commenting/FunctionCommentSniff.php
    • Based on (extends) PEAR.Commenting.FunctionComment Sniff
    • also based on Squiz.Commenting.FunctionComment Sniff
    • Reason for Custom sniff
      • Extended ruleset for parsing and verifying the doc comments for functions
      • we have a difference in our spaces to force alignment with @return
    • reviewed on 2017/02/19
  • Commenting/SingleCommentSniff.php
    • Based on PEAR.Commenting.InlineComment Sniff
    • Reason for Custom sniff
      • differences in SingleComment coding standards
    • reviewed on 2017/02/19
  • ControlStructures/ControlSignatureSniff.php
    • Based on Squiz.ControlStructures.ControlSignature sniff
    • Reason for Custom sniff
      • differences in control structure coding standards
    • reviewed on 2017/02/19
  • ControlStructures/ControlStructuresBracketsSniff.php
    • Based on PEAR.Classes.ClassDeclaration Sniff
    • Reason for Custom sniff
      • Checks if the declaration of control structures is correct Curly brackets must be on a line by their own
    • reviewed on 2017/02/14
  • ControlStructures/WhiteSpaceBeforeSniff.php
    • Based on (this seems to be a fully custom sniff)
    • Reason for Custom sniff
      • Checks that there is an empty line before a control structure or return statement
    • reviewed on 2017/02/19
  • Functions/StatementNotFunctionSniff.php
    • Based on PEAR.Files.IncludingFile Sniff
    • Reason for Custom sniff
      • add tokens so sniff covers all php statements
    • reviewed on 2017/02/19
  • NamingConventions/ValidFunctionNameSniff.php
  • NamingConventions/ValidVariableNameSniff.php
    • Based on (extends) Squiz.NamingConventions.ValidVariableName Sniff
    • Reason for Custom sniff
      • changes to processMemberVar function in the sniff to ignore the Private member variable "%s" must contain a leading underscore requierment
    • reviewed on 2017/02/19
  • Operators/ValidLogicalOperatorsSniff.php
    • Based on Squiz.ValidLogicalOperators Sniff
    • Reason for Custom sniff
      • Exceptions for things like or jexit(), or JSession, or define, or sendResponse, or sendJsonResponse
    • reviewed on 2017/02/19
  • WhiteSpace/MemberVarSpacingSniff.php

@photodude
Copy link
Contributor

photodude commented Feb 20, 2017

I have completed a review of our custom stiffs against the PHPCS core sniffs they are based on, (as best I could tell) Details of the review are posted above, and PR's have been opened for the related changes.

@photodude
Copy link
Contributor

@mbabker what is the plan on releasing the public alpha?

@photodude
Copy link
Contributor

With a partial improved exclusion list, I get the following running against the CMS. Working through the rest of the current excludes in the CMS 1.5.x ruleset, and still trying to figure out why travis is having strange issues running with the CMS specific modifications to the ruleset.

----------------------------------------------------------------------
A TOTAL OF 9553 ERRORS AND 968 WARNINGS WERE FOUND IN 1147 FILES
----------------------------------------------------------------------
PHPCBF CAN FIX 7262 OF THESE SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------

@photodude
Copy link
Contributor

photodude commented Dec 18, 2017

With almost all of the current exclusions in the CMS I get the following on local testing

----------------------------------------------------------------------
A TOTAL OF 8236 ERRORS AND 247 WARNINGS WERE FOUND IN 998 FILES
----------------------------------------------------------------------
PHPCBF CAN FIX 6965 OF THESE SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------

There are a few more exclusions to add where old custom sniffs were replaced with core sniffs

@photodude
Copy link
Contributor

photodude commented Dec 22, 2017

I've got a preliminary exclusions list now. I'm working through this list to eliminate the overly broad exclusions on some of the sniffs where I exclude all tests, components, plugins, etc so we can get to just excluding the folder or single file that has problems.

Note, this has a lot more exclusions than the 1.5.x ruleset as the 1.5.x list didn't do as good of a job catching and enforcing our code style as the 2.x ruleset does.

<?xml version="1.0"?>
<ruleset name="Joomla-CMS">
	<arg name="report" value="full"/>
	<arg name="tab-width" value="4"/>
	<arg name="encoding" value="utf-8"/>
	<arg value="sp"/>

	<!-- Exclude folders not containing production code -->
	<exclude-pattern type="relative">^build/*</exclude-pattern>
	<exclude-pattern type="relative">docs/*</exclude-pattern>
	<exclude-pattern type="relative">cache/*</exclude-pattern>
	<exclude-pattern type="relative">tmp/*</exclude-pattern>
	<exclude-pattern type="relative">logs/*</exclude-pattern>

	<!-- Exclude 3rd party libraries and Framework code. -->
	<exclude-pattern type="relative">libraries/compat/password/*</exclude-pattern>
	<exclude-pattern type="relative">libraries/fof/*</exclude-pattern>
	<exclude-pattern type="relative">libraries/idna_convert/*</exclude-pattern>
	<exclude-pattern type="relative">libraries/php-encryption/*</exclude-pattern>
	<exclude-pattern type="relative">libraries/phputf8/*</exclude-pattern>
	<exclude-pattern type="relative">libraries/simplepie/*</exclude-pattern>
	<exclude-pattern type="relative">libraries/phpass/*</exclude-pattern>
	<exclude-pattern type="relative">libraries/vendor/*</exclude-pattern>
	<exclude-pattern type="relative">libraries/joomla/*</exclude-pattern>
	<exclude-pattern type="relative">plugins/editors/*</exclude-pattern>
	<exclude-pattern type="relative">plugins/editors-xtd/*</exclude-pattern>
	<exclude-pattern type="relative">plugins/captcha/recaptcha/recaptcha.php</exclude-pattern>
	<exclude-pattern type="relative">plugins/captcha/recaptcha/recaptchalib.php</exclude-pattern>
	<exclude-pattern>*/vendor/*</exclude-pattern>
	<exclude-pattern>*/fof/*</exclude-pattern>

	<!-- Exclude the restore_finalisation until we can deal with nested class definitions -->
	<exclude-pattern type="relative">administrator/components/com_joomlaupdate/restore_finalisation.php</exclude-pattern>
	<exclude-pattern type="relative">administrator/components/com_joomlaupdate/restore.php</exclude-pattern>
	<exclude-pattern type="relative">configuration.php</exclude-pattern>
	<exclude-pattern type="relative">installation/template/index.php</exclude-pattern>
	
	<!-- Exclude some test related files that don't actually include PHP code -->
	<exclude-pattern type="relative">tests/unit/suites/libraries/joomla/model/stubs/barbaz.php</exclude-pattern>
	<exclude-pattern type="relative">tests/unit/suites/libraries/joomla/view/layouts1/fringe/division.php</exclude-pattern>
	<exclude-pattern type="relative">tests/unit/suites/libraries/joomla/view/layouts1/olivia.php</exclude-pattern>
	<exclude-pattern type="relative">tests/unit/suites/libraries/joomla/view/layouts1/peter.php</exclude-pattern>
	<exclude-pattern type="relative">tests/unit/suites/libraries/joomla/view/layouts2/fauxlivia.php</exclude-pattern>
	<exclude-pattern type="relative">tests/unit/suites/libraries/joomla/view/layouts2/olivia.php</exclude-pattern>
	<exclude-pattern type="relative">tests/unit/suites/libraries/legacy/controller/stubs/component1/controller.json.php</exclude-pattern>
	<exclude-pattern type="relative">tests/unit/suites/libraries/legacy/controller/stubs/component2/controller.php</exclude-pattern>
	
	<!-- Exclude the RoboFile.php -->
	<exclude-pattern type="relative">RoboFile.php</exclude-pattern>
	
	<!-- Include some additional sniffs from the Generic standard -->
	<rule ref="Generic.Arrays.DisallowShortArraySyntax">
		<!-- These exceptions are temporary. Remove these exceptions as code style violations are fixed -->
		<exclude-pattern type="relative">tests/codeception/*</exclude-pattern>
	</rule>
	<rule ref="Generic.ControlStructures.InlineControlStructure">
		<!-- These exceptions are temporary. Remove these exceptions as code style violations are fixed -->
		<exclude-pattern type="relative">*/tmpl/*</exclude-pattern>
	</rule>
	<rule ref="Generic.Files.EndFileNewline">
		<!-- These exceptions are temporary. Remove these exceptions as code style violations are fixed -->
		<exclude-pattern type="relative">*/tmpl/*</exclude-pattern>
		<exclude-pattern type="relative">templates/*</exclude-pattern>
		<exclude-pattern type="relative">layouts/*</exclude-pattern>
	</rule>
	<rule ref="Generic.Files.LineLength">
		<!-- These exceptions are temporary. Remove these exceptions as code style violations are fixed -->
		<exclude-pattern type="relative">*/tmpl/*</exclude-pattern>
		<exclude-pattern type="relative">templates/*</exclude-pattern>
		<exclude-pattern type="relative">layouts/*</exclude-pattern>
		<exclude-pattern type="relative">plugins/system/*</exclude-pattern>
		<exclude-pattern type="relative">plugins/search/*</exclude-pattern>
		<exclude-pattern type="relative">tests/unit/suites/libraries/cms/*</exclude-pattern>
		<exclude-pattern type="relative">tests/unit/suites/database/driver/*</exclude-pattern>
		<exclude-pattern type="relative">libraries/src/Access/Access.php</exclude-pattern>
		<exclude-pattern type="relative">libraries/src/Application/*</exclude-pattern>
		<exclude-pattern type="relative">libraries/src/Categories/Categories.php</exclude-pattern>
		<exclude-pattern type="relative">libraries/src/Renderer/*</exclude-pattern>
		<exclude-pattern type="relative">libraries/src/Client/FtpClient.php</exclude-pattern>
		<exclude-pattern type="relative">libraries/src/Component/*</exclude-pattern>
		<exclude-pattern type="relative">libraries/src/Document/Document.php</exclude-pattern>
		<exclude-pattern type="relative">libraries/src/Environment/Browser.php</exclude-pattern>
		<exclude-pattern type="relative">libraries/src/Factory.php</exclude-pattern>
		<exclude-pattern type="relative">libraries/src/Form/*</exclude-pattern>
		<exclude-pattern type="relative">libraries/src/Helper/*</exclude-pattern>
		<exclude-pattern type="relative">libraries/src/HTML/*</exclude-pattern>
		<exclude-pattern type="relative">libraries/src/Http/*</exclude-pattern>
		<exclude-pattern type="relative">libraries/src/Installer/*</exclude-pattern>
		<exclude-pattern type="relative">libraries/src/Language/*</exclude-pattern>
		<exclude-pattern type="relative">libraries/src/Log/*</exclude-pattern>
		<exclude-pattern type="relative">libraries/src/MVC/*</exclude-pattern>
		<exclude-pattern type="relative">libraries/src/Pathway/*</exclude-pattern>
		<exclude-pattern type="relative">libraries/src/Router/*</exclude-pattern>
		<exclude-pattern type="relative">libraries/src/Table/*</exclude-pattern>
		<exclude-pattern type="relative">libraries/src/Document/Renderer/Feed/AtomRenderer.php</exclude-pattern>
		<exclude-pattern type="relative">libraries/src/Document/Renderer/Html/HeadRenderer.php</exclude-pattern>
		<exclude-pattern type="relative">libraries/legacy/*</exclude-pattern>
		<exclude-pattern type="relative">libraries/cms/html/*</exclude-pattern>
		<exclude-pattern type="relative">administrator/components/com_users/*</exclude-pattern>
		<exclude-pattern type="relative">administrator/components/com_redirect/*</exclude-pattern>
		<exclude-pattern type="relative">administrator/components/com_postinstall/*</exclude-pattern>
		<exclude-pattern type="relative">administrator/components/com_newsfeeds/*</exclude-pattern>
		<exclude-pattern type="relative">administrator/components/com_modules/*</exclude-pattern>
		<exclude-pattern type="relative">administrator/components/com_menus/*</exclude-pattern>
		<exclude-pattern type="relative">administrator/components/com_media/*</exclude-pattern>
		<exclude-pattern type="relative">administrator/components/com_languages/*</exclude-pattern>
		<exclude-pattern type="relative">administrator/components/com_installer/*</exclude-pattern>
		<exclude-pattern type="relative">administrator/components/com_finder/*</exclude-pattern>
		<exclude-pattern type="relative">administrator/components/com_fields/*</exclude-pattern>
		<exclude-pattern type="relative">administrator/components/com_content/*</exclude-pattern>
		<exclude-pattern type="relative">administrator/components/com_contact/*</exclude-pattern>
		<exclude-pattern type="relative">administrator/components/com_categories/*</exclude-pattern>
		<exclude-pattern type="relative">administrator/components/com_banners/*</exclude-pattern>
		<exclude-pattern type="relative">administrator/components/com_associations/*</exclude-pattern>
		<exclude-pattern type="relative">administrator/components/com_admin/*</exclude-pattern>
		<exclude-pattern type="relative">administrator/modules/mod_latest/helper.php</exclude-pattern>
		<exclude-pattern type="relative">administrator/modules/mod_logged/helper.php</exclude-pattern>
		<exclude-pattern type="relative">components/com_finder/helpers/html/filter.php</exclude-pattern>
		<exclude-pattern type="relative">components/com_content/views/featured/view.feed.php</exclude-pattern>
		<exclude-pattern type="relative">components/com_content/views/category/view.html.php</exclude-pattern>
		<exclude-pattern type="relative">components/com_content/views/category/view.feed.php</exclude-pattern>
		<exclude-pattern type="relative">components/com_content/views/article/view.html.php</exclude-pattern>
		<exclude-pattern type="relative">components/com_content/models/article.php</exclude-pattern>
		<exclude-pattern type="relative">components/com_contact/views/contact/view.html.php</exclude-pattern>
		<exclude-pattern type="relative">components/com_tags/controller.php</exclude-pattern>
		<exclude-pattern type="relative">installation/model/database.php</exclude-pattern>
		<exclude-pattern type="relative">installation/form/field/sample.php</exclude-pattern>
		<exclude-pattern type="relative">modules/mod_articles_category/mod_articles_category.php</exclude-pattern>
		<exclude-pattern type="relative">libraries/src/Updater/Adapter/ExtensionAdapter.php</exclude-pattern>
		<exclude-pattern type="relative">libraries/src/Table/Table.php</exclude-pattern>
		<exclude-pattern type="relative">libraries/loader.php</exclude-pattern>
		<exclude-pattern type="relative">libraries/legacy/error/error.php</exclude-pattern>
		<exclude-pattern type="relative">plugins/user/profile/profile.php</exclude-pattern>
		<exclude-pattern type="relative">plugins/content/pagebreak/pagebreak.php</exclude-pattern>
		<exclude-pattern type="relative">plugins/authentication/cookie/cookie.php</exclude-pattern>
		<exclude-pattern type="relative">tests/unit/suites/plugins/content/emailcloak/PlgContentEmailcloakTest.php</exclude-pattern>
		<exclude-pattern type="relative">tests/unit/core/case/cache.php</exclude-pattern>
		<exclude-pattern type="relative">tests/unit/core/case/database/sqlsrv.php</exclude-pattern>
	</rule>
	<rule ref="Generic.Formatting.DisallowMultipleStatements">
		<!-- These exceptions are temporary. Remove these exceptions as code style violations are fixed -->
		<exclude-pattern type="relative">*/tmpl/*</exclude-pattern>
		<exclude-pattern type="relative">templates/*</exclude-pattern>
		<exclude-pattern type="relative">layouts/*</exclude-pattern>
	</rule>
	<rule ref="Generic.Strings.UnnecessaryStringConcat">
		<!-- There is not auto fixer here. These exceptions are temporary. Remove these exceptions as code style violations are fixed -->
		<exclude-pattern type="relative">*/tmpl/*</exclude-pattern>
		<exclude-pattern type="relative">templates/*</exclude-pattern>
		<exclude-pattern type="relative">layouts/*</exclude-pattern>
		<exclude-pattern type="relative">libraries/src/Helper/ContentHistoryHelper.php</exclude-pattern>
		<exclude-pattern type="relative">libraries/src/Helper/TagsHelper.php</exclude-pattern>
		<exclude-pattern type="relative">libraries/src/Table/Nested.php</exclude-pattern>
		<exclude-pattern type="relative">administrator/components/com_modules/models/module.php</exclude-pattern>
		<exclude-pattern type="relative">modules/mod_tags_popular/helper.php</exclude-pattern>
		<exclude-pattern type="relative">plugins/search/content/content.php</exclude-pattern>
		<exclude-pattern type="relative">plugins/system/debug/debug.php</exclude-pattern>
		<exclude-pattern type="relative">components/com_content/models/articles.php</exclude-pattern>
		<exclude-pattern type="relative">administrator/modules/mod_menus/models/menus.php</exclude-pattern>
		<exclude-pattern type="relative">administrator/components/com_joomlaupdate/models/default.php</exclude-pattern>
		<exclude-pattern type="relative">administrator/components/com_menus/models/menus.php</exclude-pattern>
		<exclude-pattern type="relative">administrator/components/com_menus/models/items.php</exclude-pattern>
		<exclude-pattern type="relative">administrator/components/com_associations/models/associations.php</exclude-pattern>
		<exclude-pattern type="relative">tests/unit/suites/libraries/legacy/application/JApplicationTest.php</exclude-pattern>
	</rule>
	<rule ref="Generic.WhiteSpace.ScopeIndent">
		<!-- These exceptions are temporary. Remove these exceptions as code style violations are fixed -->
		<exclude-pattern type="relative">*/tmpl/*</exclude-pattern>
		<exclude-pattern type="relative">templates/*</exclude-pattern>
		<exclude-pattern type="relative">layouts/*</exclude-pattern>
	</rule>
	
	<!-- Include some additional sniffs from the PEAR standard -->
	<rule ref="PEAR.ControlStructures.MultiLineCondition">
		<!-- These exceptions are temporary. Remove these exceptions as code style violations are fixed -->
		<exclude-pattern type="relative">*/tmpl/*</exclude-pattern>
		<exclude-pattern type="relative">templates/*</exclude-pattern>
		<exclude-pattern type="relative">layouts/*</exclude-pattern>
	</rule>
	<rule ref="PEAR.Formatting.MultiLineAssignment">
		<!-- These exceptions are temporary. Remove these exceptions as code style violations are fixed -->
		<exclude-pattern type="relative">plugins/system/languagefilter/languagefilter.php</exclude-pattern>
	</rule>
	<rule ref="PEAR.Functions.FunctionCallSignature">
		<!-- These exceptions are temporary. Remove these exceptions as code style violations are fixed -->
		<exclude-pattern type="relative">*/tmpl/*</exclude-pattern>
		<exclude-pattern type="relative">templates/*</exclude-pattern>
		<exclude-pattern type="relative">layouts/*</exclude-pattern>
	</rule>
	<rule ref="PEAR.Functions.FunctionDeclaration">
		<!-- These exceptions are temporary. Remove these exceptions as code style violations are fixed -->
		<exclude-pattern type="relative">*/tmpl/*</exclude-pattern>
		<exclude-pattern type="relative">templates/*</exclude-pattern>
		<exclude-pattern type="relative">layouts/*</exclude-pattern>
	</rule>
	<rule ref="PEAR.Functions.ValidDefaultValue">
		<!-- These exceptions are temporary. Remove these exceptions as code style violations are fixed -->
		<exclude-pattern type="relative">administrator/components/com_contact/helpers/html/contact.php</exclude-pattern>
		<exclude-pattern type="relative">administrator/components/com_content/helpers/html/contentadministrator.php</exclude-pattern>
		<exclude-pattern type="relative">libraries/cms/component/router/stubs/ComContentRouter.php</exclude-pattern>
		<exclude-pattern type="relative">libraries/cms/pagination/JPaginationObjectTest.php</exclude-pattern>
		<exclude-pattern type="relative">modules/mod_articles_category/helper.php</exclude-pattern>
	</rule>
	<rule ref="PEAR.NamingConventions.ValidClassName">
		<!-- These exceptions are temporary. Remove these exceptions as code style violations are fixed -->
		<exclude-pattern type="relative">installation/controller/install/database_backup.php</exclude-pattern>
		<exclude-pattern type="relative">installation/controller/install/database_remove.php</exclude-pattern>
	</rule>
	
	<!-- Include some additional sniffs from the Squiz standard -->
	<rule ref="Squiz.Commenting.BlockComment">
		<!-- These exceptions are temporary. Remove these exceptions as code style violations are fixed -->
		<exclude-pattern type="relative">*/tmpl/*</exclude-pattern>
		<exclude-pattern type="relative">templates/*</exclude-pattern>
		<exclude-pattern type="relative">plugins/editors/*</exclude-pattern>
		<exclude-pattern type="relative">layouts/*</exclude-pattern>
	</rule>
	<rule ref="Squiz.Commenting.VariableComment">
		<!-- These exceptions are temporary. Remove these exceptions as code style violations are fixed -->
		<exclude-pattern type="relative">tests/unit/core/mock/menu.php</exclude-pattern>
		<exclude-pattern type="relative">tests/unit/stubs/bogusload.php</exclude-pattern>
		<exclude-pattern type="relative">tests/unit/stubs/config.wrongclass.php</exclude-pattern>
		<exclude-pattern type="relative">tests/unit/suites/database/driver/mysql/JDatabaseExporterMysqlTest.php</exclude-pattern>
		<exclude-pattern type="relative">tests/unit/suites/libraries/cms/application/stubs/JApplicationCmsInspector.php</exclude-pattern>
		<exclude-pattern type="relative">tests/unit/suites/libraries/cms/component/router/stubs/ComContentRouter.php</exclude-pattern>
		<exclude-pattern type="relative">tests/unit/suites/libraries/cms/html/testfiles/inspector.php</exclude-pattern>
		<exclude-pattern type="relative">tests/unit/suites/libraries/cms/html/TestHelpers/JHtmlSelect-helper-dataset.php</exclude-pattern>
		<exclude-pattern type="relative">tests/unit/suites/libraries/cms/installer/JInstallerAdapterTest.php</exclude-pattern>
		<exclude-pattern type="relative">tests/unit/suites/libraries/legacy/view/JViewLegacyTest.php</exclude-pattern>
		<exclude-pattern type="relative">plugins/content/loadmodule/loadmodule.php</exclude-pattern>
		<exclude-pattern type="relative">plugins/system/debug/debug.php</exclude-pattern>
		<exclude-pattern type="relative">components/com_users/views/*</exclude-pattern>
		<exclude-pattern type="relative">components/com_tags/views/*</exclude-pattern>
		<exclude-pattern type="relative">components/com_newsfeeds/helpers/*</exclude-pattern>
		<exclude-pattern type="relative">components/com_redirect/helpers/*</exclude-pattern>
		<exclude-pattern type="relative">components/com_redirect/views/*</exclude-pattern>
		<exclude-pattern type="relative">components/com_search/views/*</exclude-pattern>
		<exclude-pattern type="relative">libraries/src/Form/Field/MediaField.php</exclude-pattern>
		<exclude-pattern type="relative">libraries/src/Menu/Node.php</exclude-pattern>
		<exclude-pattern type="relative">libraries/src/MVC/View/CategoryView.php</exclude-pattern>
		<exclude-pattern type="relative">administrator/components/com_plugins/*</exclude-pattern>
		<exclude-pattern type="relative">administrator/components/com_installer/models/database.php</exclude-pattern>
		<exclude-pattern type="relative">administrator/components/com_content/helpers/content.php</exclude-pattern>
		<exclude-pattern type="relative">administrator/components/com_contenthistory/views/*</exclude-pattern>
		<exclude-pattern type="relative">administrator/components/com_templates/views/*</exclude-pattern>
		<exclude-pattern type="relative">administrator/components/com_messages/views/*</exclude-pattern>
		<exclude-pattern type="relative">administrator/components/com_installer/views/*</exclude-pattern>
		<exclude-pattern type="relative">administrator/components/com_languages/views/*</exclude-pattern>
		<exclude-pattern type="relative">administrator/components/com_messages/models/*</exclude-pattern>
		<exclude-pattern type="relative">administrator/components/com_fields/models/*</exclude-pattern>
		<exclude-pattern type="relative">administrator/components/com_fields/libraries/*</exclude-pattern>
		<exclude-pattern type="relative">administrator/components/com_fields/helpers/*</exclude-pattern>
		<exclude-pattern type="relative">administrator/components/com_fields/controllers/*</exclude-pattern>
		<exclude-pattern type="relative">administrator/components/com_categories/helpers/*</exclude-pattern>
		<exclude-pattern type="relative">administrator/components/com_associations/helpers/associations.php</exclude-pattern>
		<exclude-pattern type="relative">administrator/components/com_menus/helpers/menus.php</exclude-pattern>
		<exclude-pattern type="relative">components/com_config/view/*</exclude-pattern>
		<exclude-pattern type="relative">components/com_contact/views/*</exclude-pattern>
		<exclude-pattern type="relative">components/com_modules/views/*</exclude-pattern>
		<exclude-pattern type="relative">components/com_contact/models/*</exclude-pattern>
		<exclude-pattern type="relative">components/com_content/models/*</exclude-pattern>
		<exclude-pattern type="relative">components/com_tags/helpers/route.php</exclude-pattern>
		<exclude-pattern type="relative">components/com_newsfeeds/models/category.php</exclude-pattern>
		<exclude-pattern type="relative">components/com_newsfeeds/models/categories.php</exclude-pattern>
		<exclude-pattern type="relative">components/com_finder/views/search/view.html.php</exclude-pattern>
		<exclude-pattern type="relative">components/com_content/views/form/view.html.php</exclude-pattern>
		<exclude-pattern type="relative">components/com_content/views/featured/view.html.php</exclude-pattern>
		<exclude-pattern type="relative">components/com_content/views/article/view.html.php</exclude-pattern>
		<exclude-pattern type="relative">components/com_content/views/archive/view.html.php</exclude-pattern>
		<exclude-pattern type="relative">components/com_contact/views/contact/view.html.php</exclude-pattern>
		<exclude-pattern type="relative">components/com_contact/views/catagory/view.html.php</exclude-pattern>
		<exclude-pattern type="relative">components/com_contact/models/featured.php</exclude-pattern>
		<exclude-pattern type="relative">components/com_contact/models/contact.php</exclude-pattern>
		<exclude-pattern type="relative">components/com_contact/models/catagory.php</exclude-pattern>
		<exclude-pattern type="relative">components/com_contact/models/catagories.php</exclude-pattern>
		<exclude-pattern type="relative">components/com_contact/router.php</exclude-pattern>
		<exclude-pattern type="relative">components/com_content/router.php</exclude-pattern>
		<exclude-pattern type="relative">components/com_newsfeeds/router.php</exclude-pattern>
		<exclude-pattern type="relative">components/com_content/models/catagory.php</exclude-pattern>
		<exclude-pattern type="relative">components/com_content/models/catagories.php</exclude-pattern>
		<exclude-pattern type="relative">libraries/src/User/User.php</exclude-pattern>
		<exclude-pattern type="relative">libraries/src/Updater/UpdateAdapter.php</exclude-pattern>
		<exclude-pattern type="relative">libraries/src/Updater/Update.php</exclude-pattern>
		<exclude-pattern type="relative">libraries/cms/less/formatter/joomla.php</exclude-pattern>
		<exclude-pattern type="relative">administrator/modules/mod_menu/menu.php</exclude-pattern>
		<exclude-pattern type="relative">administrator/modules/mod_quickicon/helper.php</exclude-pattern>
	</rule>
	<rule ref="Squiz.Strings.ConcatenationSpacing">
		<!-- These exceptions are temporary. Remove these exceptions as code style violations are fixed -->
		<exclude-pattern type="relative">*/tmpl/*</exclude-pattern>
		<exclude-pattern type="relative">templates/*</exclude-pattern>
		<exclude-pattern type="relative">layouts/*</exclude-pattern>
	</rule>
	<rule ref="Squiz.WhiteSpace.ControlStructureSpacing">
		<!-- These exceptions are temporary. Remove these exceptions as code style violations are fixed -->
		<exclude-pattern type="relative">*/tmpl/*</exclude-pattern>
		<exclude-pattern type="relative">templates/*</exclude-pattern>
		<exclude-pattern type="relative">layouts/*</exclude-pattern>
	</rule>
	<rule ref="Squiz.WhiteSpace.OperatorSpacing">
		<!-- These exceptions are temporary. Remove these exceptions as code style violations are fixed -->
		<exclude-pattern type="relative">*/tmpl/*</exclude-pattern>
		<exclude-pattern type="relative">templates/*</exclude-pattern>
		<exclude-pattern type="relative">layouts/*</exclude-pattern>
	</rule>
	<rule ref="Squiz.WhiteSpace.ScopeClosingBrace">
		<!-- These exceptions are temporary. Remove these exceptions as code style violations are fixed -->
		<exclude-pattern type="relative">*/tmpl/*</exclude-pattern>
		<exclude-pattern type="relative">templates/*</exclude-pattern>
		<exclude-pattern type="relative">layouts/*</exclude-pattern>
	</rule>
	
	<!-- Include some additional sniffs from the Zend standard -->
	<rule ref="Zend.Files.ClosingTag">
		<!-- These exceptions are temporary. Remove these exceptions as code style violations are fixed -->
		<exclude-pattern type="relative">*/tmpl/*</exclude-pattern>
		<exclude-pattern type="relative">templates/*</exclude-pattern>
		<exclude-pattern type="relative">layouts/*</exclude-pattern>
	</rule>
	
	<!-- CMS specific sniff exclusions from the Joomla standard -->
	<rule ref="Joomla.Commenting.FileComment">
		<!-- These exceptions are temporary. Remove these exceptions as code style violations are fixed -->
		<exclude-pattern type="relative">tests/*</exclude-pattern>
	</rule>
	<rule ref="Joomla.Commenting.FunctionComment">
		<!-- These exceptions are temporary. Remove these exceptions as code style violations are fixed -->
		<exclude-pattern type="relative">templates/*</exclude-pattern>
		<exclude-pattern type="relative">layouts/*</exclude-pattern>
		<exclude-pattern type="relative">modules/*</exclude-pattern>
		<exclude-pattern type="relative">plugins/*</exclude-pattern>
		<exclude-pattern type="relative">*/tests/*</exclude-pattern>
		<exclude-pattern type="relative">libraries/legacy/*</exclude-pattern>
		<exclude-pattern type="relative">libraries/cms/html/*</exclude-pattern>
		<exclude-pattern type="relative">installation/model/*</exclude-pattern>
		<exclude-pattern type="relative">installation/application/*</exclude-pattern>
		<exclude-pattern type="relative">libraries/src/Application/CMSApplication.php</exclude-pattern>
		<exclude-pattern type="relative">libraries/src/Application/DaemonApplication.php</exclude-pattern>
		<exclude-pattern type="relative">libraries/src/Application/WebApplication.php</exclude-pattern>
		<exclude-pattern type="relative">libraries/src/Captcha/Captcha.php</exclude-pattern>
		<exclude-pattern type="relative">libraries/src/Client/FtpClient.php</exclude-pattern>
		<exclude-pattern type="relative">libraries/src/Utility/BufferStreamHandler.php</exclude-pattern>
		<exclude-pattern type="relative">libraries/src/User/User.php</exclude-pattern>
		<exclude-pattern type="relative">libraries/src/Updater/UpdateAdapter.php</exclude-pattern>
		<exclude-pattern type="relative">libraries/src/Toolbar/ToolbarButton.php</exclude-pattern>
		<exclude-pattern type="relative">libraries/src/Toolbar/Toolbar.php</exclude-pattern>
		<exclude-pattern type="relative">libraries/src/Toolbar/Button/SeparatorButton.php</exclude-pattern>
		<exclude-pattern type="relative">libraries/loader.php</exclude-pattern>
		<exclude-pattern type="relative">libraries/legacy/error/error.php</exclude-pattern>
		<exclude-pattern type="relative">libraries/src/Component/Router/*</exclude-pattern>
		<exclude-pattern type="relative">libraries/src/Document/*</exclude-pattern>
		<exclude-pattern type="relative">libraries/src/Editor/Editor.php</exclude-pattern>
		<exclude-pattern type="relative">libraries/src/Environment/Browser.php</exclude-pattern>
		<exclude-pattern type="relative">libraries/src/Filter/Wrapper/OutputFilterWrapper.php</exclude-pattern>
		<exclude-pattern type="relative">libraries/src/Form/*</exclude-pattern>
		<exclude-pattern type="relative">libraries/src/Helper/*</exclude-pattern>
		<exclude-pattern type="relative">libraries/src/Input/*</exclude-pattern>
		<exclude-pattern type="relative">libraries/src/Installer/*</exclude-pattern>
		<exclude-pattern type="relative">libraries/src/Language/*</exclude-pattern>
		<exclude-pattern type="relative">libraries/src/Log/*</exclude-pattern>
		<exclude-pattern type="relative">libraries/src/Menu/*</exclude-pattern>
		<exclude-pattern type="relative">libraries/src/MVC/*</exclude-pattern>
		<exclude-pattern type="relative">libraries/src/Plugin/*</exclude-pattern>
		<exclude-pattern type="relative">libraries/src/Router/*</exclude-pattern>
		<exclude-pattern type="relative">libraries/src/Session/Session.php</exclude-pattern>
		<exclude-pattern type="relative">libraries/src/String/PunycodeHelper.php</exclude-pattern>
		<exclude-pattern type="relative">libraries/src/Table/*</exclude-pattern>
		<exclude-pattern type="relative">libraries/src/HTML/HTMLHelper.php</exclude-pattern>
		<exclude-pattern type="relative">libraries/src/Layout/FileLayout.php</exclude-pattern>
		<exclude-pattern type="relative">components/com_wrapper/views/wrapper/view.html.php</exclude-pattern>
		<exclude-pattern type="relative">administrator/components/*</exclude-pattern>
		<exclude-pattern type="relative">components/com_banners/router.php</exclude-pattern>
		<exclude-pattern type="relative">components/com_config/*</exclude-pattern>
		<exclude-pattern type="relative">components/com_contact/*</exclude-pattern>
		<exclude-pattern type="relative">components/com_content/*</exclude-pattern>
		<exclude-pattern type="relative">components/com_finder/*</exclude-pattern>
		<exclude-pattern type="relative">components/com_newsfeeds*</exclude-pattern>
		<exclude-pattern type="relative">components/com_search/*</exclude-pattern>
		<exclude-pattern type="relative">components/com_tags/*</exclude-pattern>
		<exclude-pattern type="relative">components/com_users/*</exclude-pattern>
		<exclude-pattern type="relative">components/com_wrapper/router.php</exclude-pattern>
	</rule>
	<rule ref="Joomla.Commenting.SingleComment">
		<!-- These exceptions are temporary. Remove these exceptions as code style violations are fixed -->
		<exclude-pattern type="relative">*/tmpl/*</exclude-pattern>
		<exclude-pattern type="relative">templates/*</exclude-pattern>
		<exclude-pattern type="relative">layouts/*</exclude-pattern>
	</rule>
	<rule ref="Joomla.Commenting.ClassComment">
		<!-- These exceptions are temporary. Remove these exceptions as code style violations are fixed -->
		<exclude-pattern type="relative">tests/*</exclude-pattern>
	</rule>
	<rule ref="Joomla.ControlStructures.ControlSignature">
		<!-- These exceptions are temporary. Remove these exceptions as code style violations are fixed -->
		<exclude-pattern type="relative">*/tmpl/*</exclude-pattern>
		<exclude-pattern type="relative">templates/*</exclude-pattern>
		<exclude-pattern type="relative">layouts/*</exclude-pattern>
	</rule>
	<rule ref="Joomla.ControlStructures.ControlStructuresBrackets">
		<!-- These exceptions are temporary. Remove these exceptions as code style violations are fixed -->
		<exclude-pattern type="relative">*/tmpl/*</exclude-pattern>
		<exclude-pattern type="relative">templates/*</exclude-pattern>
		<exclude-pattern type="relative">layouts/*</exclude-pattern>
	</rule>
	<rule ref="Joomla.ControlStructures.WhiteSpaceBefore">
		<!-- These exceptions are temporary. Remove these exceptions as code style violations are fixed -->
		<exclude-pattern type="relative">*/tmpl/*</exclude-pattern>
		<exclude-pattern type="relative">templates/*</exclude-pattern>
		<exclude-pattern type="relative">layouts/*</exclude-pattern>
	</rule>
	<rule ref="Joomla.Operators.ValidLogicalOperators">
		<!-- These exceptions are temporary. Remove these exceptions as code style violations are fixed -->
		<exclude-pattern type="relative">*/tmpl/*</exclude-pattern>
		<exclude-pattern type="relative">templates/*</exclude-pattern>
		<exclude-pattern type="relative">layouts/*</exclude-pattern>
	</rule>
	<rule ref="Joomla">
		<!-- These exceptions are permanent as long as there are B/C naming exceptions  -->
		<exclude name="Joomla.NamingConventions.ValidFunctionName.FunctionNoCapital"/>
		<exclude name="Joomla.NamingConventions.ValidFunctionName.MethodUnderscore"/>
		<exclude name="Joomla.NamingConventions.ValidFunctionName.ScopeNotCamelCaps"/>
		<exclude name="Joomla.NamingConventions.ValidFunctionName.FunctionNameInvalid"/>
		<exclude name="Joomla.NamingConventions.ValidVariableName.ClassVarHasUnderscore"/>
		<exclude name="Joomla.NamingConventions.ValidVariableName.MemberNotCamelCaps"/>
		<exclude name="Joomla.NamingConventions.ValidVariableName.NotCamelCaps"/>
		<exclude name="Joomla.NamingConventions.ValidVariableName.StringNotCamelCaps"/>
		<exclude name="Generic.Files.LineEndings.InvalidEOLChar"/>
	</rule>
</ruleset>

@Paladin
Copy link

Paladin commented Dec 22, 2017

OK, I think I'm missing something essential to understand this list of exclusions:

Are this many files really being enshrined as exempt from the project's code style? Aside from the third party libraries, for which I can readily grasp reasons for excluding, seeing this many of the project's own files excluded brings up my eyebrows and and questions about either the purpose or the correctness of the code style itself.

@photodude
Copy link
Contributor

photodude commented Dec 22, 2017

@Paladin This just shows how far off the 3.x CMS Repo is from being in compliance. Often it's a single section or a single file that is the issue (I've tried to pinpoint some of those with exclusions for those single files and folders).
The Exclusions here are to mute the errors while we transition from the 1.5.x ruleset to the 2.x ruleset.
The 1.5.x CMS ruleset has a large but smaller set of exclusions than what's shown here.

This exclusion list should be much easier to reduce as some of these exceptions are now connected to rules with autofixers. Here is an example PR I've done showing how I prefer to apply the autofixers. Rather than just fixing everything and causing issues across the repo, I focus the PR's on just on small area like a single component, a single module, a single plugin, etc.

The harder area to deal with are things that cannot be autofixed and have to be addressed manually, such as line length or adding member var docblocks.

@photodude
Copy link
Contributor

photodude commented Dec 22, 2017

Personally I think it would be great if we could get a group to coordinate PR's like the example shown to burn through all of the autofixers. Then we could just remove those exclusions.
(note: the mixed PHP / HTML files will continue to be exceptions with some rules)

PHPCS command line example process

  1. check out staging as new branch
  2. run PHPCS to check for errors
php phpcs --standard=Joomla-CMS --extensions=php path\to\joomla-cms\Area\To\Check\And\Fix\
  1. copy list of rules that are autofixer capable for commit history
  2. run PHPCBF
php phpcbf -ps --no-patch --standard=Joomla-CMS --extensions=php path\to\joomla-cms\Area\To\Check\And\Fix\
  1. commit to github
  2. create PR as shown in example
  3. Rinse and Repeat.
  4. remove PHPCS exclusion related to path\to\joomla-cms\Area\To\Check\And\Fix\

@mbabker
Copy link
Contributor Author

mbabker commented Dec 22, 2017

Sooner or later I think there are some rules we need to review more closely too versus just continuing to accept the status quo unchallenged.

https://github.com/joomla/framework.joomla.org/blob/e8207b69a32d9334a13b08188ce46a04b100601d/src/Helper/GitHubHelper.php#L100
https://github.com/joomla/jissues/blob/aa3997c0f7814abc3075c49d63f5c1788dd6aaf5/src/JTracker/View/Renderer/TrackerExtension.php#L377-L379

The 150 character hard limit causes either a forced concatenation of text or you to change the project to allow longer lines to work around being forced to concatenate text in unique ways (and I don't know about others, but sometimes I really dislike being forced to fit my line length to an arbitrary limit just to appease an automated sniffer when there is no gain to be had by doing so).

@photodude
Copy link
Contributor

photodude commented Dec 22, 2017

Agreed. There is also the issue that the current code style is heavily influenced by PEAR standards and would likely benefit by adopting more PSR2 standard styling.

I will note that at first glance this item looks like it could be fixed with more SQL chaining but maybe the API lacks INSERT INTO and VALUES methods to make that happen.

The second example is definitely something I agree with. It's one of those trade offs in the code style.

side note: this code below the second example could be a one-liner.

@mbabker
Copy link
Contributor Author

mbabker commented Dec 22, 2017

I will note that at first glance this item looks like it could be fixed with more SQL chaining but maybe the API lacks INSERT INTO and VALUES methods to make that happen.

Actually it's because our API doesn't natively support MySQL's ON DUPLICATE KEY UPDATE statement so the entirety of the query needs to be written as a raw query versus using the builder. Which I'd say is expected behavior unless we can build into the builder API agnostic support for that (as in the other drivers support a similar structure too).

@photodude
Copy link
Contributor

I've opened a PR with the updates to the CMS exclusion example. I was able to clean up cases where things were too broad, although I think some of these exclusions are still too broad. (a few cases of plugins/* which could be a bit more specific)

@photodude
Copy link
Contributor

photodude commented Dec 24, 2017

It might be a few days before I can work more on this, I did get 13 more PR's opened with the CMS to address some of the things that can be fixed with the Autofixers. Once those are merged we can shrink the CMS exclusions list a little bit.

@wilsonge
Copy link
Contributor

You're not working over christmas :O outrageous :) I'll go through your PR's now :)

@Paladin
Copy link

Paladin commented Dec 26, 2017

@mbabker Ah, yes. I wondered about the necessity for that line of code when you pointed to it; thanks for the extra information. Yet one more example of how MySQL's name is misleading; it has only a coincidental relationship with SQL, which has defined the MERGE statement for that purpose since before Joomla existed.

There is probably not a good way to translate a standard query into the non-standard version in the MySQL driver without being very inefficient in mySQL's non-standard version of it.

@photodude
Copy link
Contributor

I've added another 6 PR's. I'll see when I can get back to this before I'm off my break

@photodude
Copy link
Contributor

photodude commented Jan 8, 2018

PR #218 fixes the control structure brackets indent issue within closures and has a test to verify that it fixes the issue.
Now that #218 is merged, I think we should be good to go with a stable release.

As for moving to PHPCS 2.x on the CMS 3.x and PHP3.x on CMS 4.x branches. There are some 6 PR's for CMS 3.x open for cleaning up things https://github.com/joomla/joomla-cms/pulls/photodude

@wilsonge
Copy link
Contributor

Merged 3 of them just now. In the libraries folder I was interested by the new lines after code coverage ignore comments. Maybe I'm just being a bit overreactive - but seems like new lines might be actively non-desirable there??

@photodude
Copy link
Contributor

For now let us strip the code coverage annotations out of the code.
I doubt there is a way to ignore a specific case like that without creating a custom sniff

@photodude
Copy link
Contributor

With #218 and #219 merged is there any reason not to do a stable release of the coding standards?
(I'm considering CMS compliance and use as a separate case of ongoing PRs unrelated to the actual stability of the coding standards)

@photodude
Copy link
Contributor

photodude commented Feb 9, 2018

We need to finalize our "stable" release of the PHPCS 2.x version. The next release of PHPCS 2.9.x will be the last bug fix for that branch. Note #227 for consideration with the PHPCS 2.x version.

Once our PHPCS 2.x "stable" version is released we need to move the PHPCS 3.x dev branch to master and make a release.

@mbabker
Copy link
Contributor Author

mbabker commented Feb 10, 2018

Just finished switching over the Database package. This build should keep someone busy for a few minutes 😉

https://travis-ci.org/joomla-framework/database/jobs/339728069

@photodude
Copy link
Contributor

photodude commented Feb 10, 2018

Interesting the code comment is being included by the sniff as part of the type
New issue opened #228

@photodude
Copy link
Contributor

I have a fix for the return type issues in the database package... #229
The fix works and the new tests pass... but there is an unresolvable code style issue with the multi-line if statements in the fix that I'm trying to get resolved by PHPCS.

@wilsonge
Copy link
Contributor

I've tagged a 2.0 RC https://github.com/joomla/coding-standards/releases/tag/2.0.0-rc :) Let's get the rest of the CMS and framework packages in compliance before we tag a stable :)

@mbabker
Copy link
Contributor Author

mbabker commented Feb 25, 2018

Finally all ~90 Framework package branches are done. Until the next masochistic mass update is needed...

@wilsonge
Copy link
Contributor

OK so @photodude what's the status of the CMS?

@photodude
Copy link
Contributor

photodude commented Feb 27, 2018

I was hoping to work on it about 2 weeks ago before I got hit with a very bad stomach flu. I'm finally better but now playing catch up on everything I should have gotten done.

Currently, we have two choices with the CMS

  1. more PR's to fix things that are now detected and reported. (2.x does a better job catching things than 1.5.x)
    or
  2. expand the exclusions list. (see in progress list above)

The administrator section is nearly in compliance, although there are some indenting issues from recent commits that the 1.5.x ruleset didn't catch.
I think the components section has a little bit of work needing to be done.
The biggest problem areas are libraries and tests.

This is a travis report with the larger exclusions list (although not as comprehensive as the one listed above) https://travis-ci.org/photodude/joomla-cms/builds/346905854

It's a bit slow getting the CMS in compliance as I feel the PR's need to be small to avoid disrupting all of the nearly 400 open PR's in the CMS. I've tried to keep the number of files under 20 in each PR both for review and to limit disruptions and merge conflicts.

Last few PR's I've been targeting specific rules to eliminate exclusions. As such, we no longer have any exclusions on space indenting. All thanks to the last 3 PR's we are now completely enforcing tab indenting.

I think it will be a few more days before I can bust out some more PR's to see how far we are off at the moment.

@wilsonge
Copy link
Contributor

wilsonge commented Mar 7, 2018

Ok, so I’m totally happy excluding the entire tests folder. The components folder I guess it depends what we’re excluding. But I really need to get this into the 4.x branch because we’re doing anonymous classes and it’s all going wrong with the current rule set xD

@photodude
Copy link
Contributor

If I set up the source report correctly this is a list of the current failures by frequency of errors. I expanded the existing exclusions to exclude the tests folder for this report. As you can see some of these are manual only changes

------------------------------------------------------------------------------
[ ] Joomla.Commenting.FunctionComment.MissingParamTag                    289
[ ] Joomla.Commenting.FunctionComment.ParamNameNoMatch                   279
[x] PEAR.Functions.FunctionCallSignature.Indent                          64
[ ] Joomla.Commenting.FunctionComment.InvalidReturnNotVoid               46
[x] Joomla.Commenting.FunctionComment.InvalidReturn                      45
[ ] Joomla.Commenting.FunctionComment.InvalidNoReturn                    38
[x] Squiz.Commenting.BlockComment.SingleLine                             26
[x] PEAR.Functions.FunctionCallSignature.CloseBracketLine                23
[ ] Joomla.Commenting.FunctionComment.InvalidReturnVoid                  21
[x] PEAR.WhiteSpace.ObjectOperatorIndent.Incorrect                       18
[ ] Joomla.Commenting.FunctionComment.WrongStyle                         15
[x] PEAR.Functions.FunctionDeclaration.CloseBracketLine                  11
[x] Generic.WhiteSpace.ScopeIndent.Incorrect                             10
[ ] Joomla.Commenting.FunctionComment.Missing                            8
[x] Squiz.WhiteSpace.SuperfluousWhitespace.EndLine                       7
[x] Squiz.Commenting.BlockComment.WrongEnd                               7
[x] Joomla.ControlStructures.WhiteSpaceBefore.SpaceBefore                6
[x] PEAR.Functions.FunctionDeclaration.SpaceAfterFunction                6
[ ] Squiz.Commenting.BlockComment.CloserSameLine                         6
[ ] Joomla.Commenting.SingleComment.SameLine                             6
[x] Generic.ControlStructures.InlineControlStructure.NotAllowed          5
[x] Squiz.WhiteSpace.ControlStructureSpacing.NoLineAfterClose            5
[x] PEAR.Functions.FunctionCallSignature.SpaceBeforeOpenBracket          4
[ ] Joomla.Commenting.FunctionComment.MissingReturn                      4
[x] Squiz.Commenting.BlockComment.NoNewLine                              3
[x] PEAR.ControlStructures.MultiLineCondition.StartWithBoolean           2
[x] Joomla.Commenting.SingleComment.LowerCaseAfterSentenceEnd            2
[ ] Joomla.Commenting.FunctionComment.SpacingAfter                       2
[x] Joomla.ControlStructures.ControlStructuresBrackets.SpaceBeforeBrace  1
[x] PEAR.Functions.FunctionDeclaration.SpaceBeforeOpenParen              1
[x] Squiz.WhiteSpace.OperatorSpacing.SpacingAfter                        1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants