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

Which specific coding standards are enforced? #1789

Closed
cagross opened this issue Aug 10, 2019 · 5 comments
Closed

Which specific coding standards are enforced? #1789

cagross opened this issue Aug 10, 2019 · 5 comments

Comments

@cagross
Copy link

cagross commented Aug 10, 2019

Hello. I realize WordPress Coding Standards is a set of rules for phpcs that can help detect violations of WordPress coding standards. But to be clear, is your set of rules to help discover violations of just WordPress PHP coding standards? Or WordPress PHP, JavaScript, and CSS coding standards? Or something else?

phpcs claims to enforce violations of PHP, JavaScript, and CSS code standards. And no part of your documentation seems to indicate otherwise. So my assumption is that the rules set in WordPress Coding Standards also flags violations of WordPress PHP, JavaScript, and CSS coding standards. But I just thought I'd ask before proceeding.

Thanks.

@TonyGravagno
Copy link

Your point is well-taken that it seems like CSS and JS are not well-represented explicitly here, even in documentation. Your inquiry here compelled me to spend an entire day researching exactly what WPCS does and does not do. It was time well-spent. Thanks. I'm hoping this thread can be a sort of "state-of-the-art" post from this point-forward. All of the other notes on this topic that I could find are 2+ years old.

;TL/DR; : It looks to me like WPCS does not and probably will not sniff for CSS or JS. The last recommendation in this tracker that I see is to use Stylelint and ESLint plugin.

From #1028 (comment) :

PHPCS is capable of sniffing PHP, JS and CSS files and has natively 70 sniffs available which can detect & fix issues in more than just PHP files (or specifically target one of the other types).
WPCS uses quite a few of these sniffs.
WPCS also contains a few sniffs which have specific features targetting JS and/or CSS ( VIP.AdminBarRemoval and the upcoming Classes.ClassInstantiation).

With a quick search I only found two sniffs that uses the CSS tokenizer, two for JS, and no reference to Squiz.CSS sniffs. However, it's possible that some of the sniffs being referenced from this codebase, but not themselves in this codebase, do use the CSS or JS tokenizers. Running a sniff from the command line with the -s option may help to reveal them.

I believe the following references would be of help (well, they were for me anyway) to clarify where this project is on the topic :

https://github.com/WPTRT/WPThemeReview rules (sniffs) to enforce WordPress theme review coding conventions

https://github.com/WPTRT/theme-sniffer plugin for WPTR plugin

Fascinating thread on what WPCS should include #1157 and a follow-up #1269. I include this because I don't see any mention of separating out PHP, JS, and CSS into their own set of sniffs within each proposed category - but I personally think that would be good.

From this blog referenced in this core.trac issue :

Working as a WordPress Engineer at Inpsyde—Germany’s biggest WordPress agency—I developed eslint-config-inpsyde, an ESLint config file specifically designed to follow current best practices and, at the same time, conform to the WordPress Coding Standards as best as possible.

Issue #39 was closed in 2013 on the note that "using a JavaScript-specific tool may be better, although it would require more configuration to get set up."

In 2017 #644 concluded:

I think we're better off deferring to Stylelint and ESLint, instead of trying to replicate all the checks in PHPCS, so I'll mark this as closed.

Stylelint is described:

Configuration rules to ensure your CSS is compliant with the WordPress CSS Coding Standards.

Stylelint is still active but the "WordPress-Coding-Standards/eslint-config-wordpress" repo has been archived, now recommending this NPMJS package: "ESLint plugin including configurations and custom rules for WordPress development."

My takeaway from this research:

It's great that there are standards defined for Core, but the tooling is prohibitively complicated. It's no wonder that so many plugin and theme developers don't follow standards. As much as I'd like some of my FOSS contribution to include reformatting some plugins to WPCS, I dare not do so, because it would impose a requirement for other developers to use the same tools that I adopt to format their code, which most people will simply not do.

What I'd like to see from the WPCS maintainers is a clear statement of recommended tooling so that more projects can get closer to the ideal. If recommendations for tooling change, so be it. But that would be done with the same consideration and care as any core change, with discussion about how it would impact developers and their existing code. The tooling is even largely unimportant. What's important is that non-core developers adopt their own preferred tooling which results in more consistent code across the industry.

Why would I like to see that? Clean and consistent code is easier for anyone to read. Consistency across packages reduces the burden of new developer interest in each new project and file. That can help with adoption of dead plugins, which an industry plague. The WPCS not only provides the core standards for improved security, it goes on to provide guidance on best practices. This is an excellent way to help non-core developers to learn how to code better. Better code can lead to plugin developers getting more help from others in the spirit of FOSS, whether in the form of code or code documentation. That collaboration might help to further encourage some of these plugin developers, and keep some plugins alive for longer. That tangibly serves this industry in the form of lower cost of ownership.

@cagross
Copy link
Author

cagross commented Sep 11, 2019

@TonyGravagno OK thanks for that. I'll just quickly throw in a test I did. In VSCode I activated phpcs and WPCS. When I open one of my PHP files in VSCode, I can see many WPCS errors and warnings. In a CSS file, I then opened one of my CSS files in VSCode, and made a change that violates one of the WordPress CSS coding standards (0 values should not have units unless necessary, such as with transition-duration), and no warning/error was displayed in VSCode.

@dingo-d
Copy link
Member

dingo-d commented Sep 11, 2019

Currently there are two WPCS sniffs that affect CSS:
I18nTextDomainFixerSniff and PrecisionAlignmentSniff.

For JS, besides the PrecisionAlignmentSniff there is also ClassInstantiationSniff.

There are probably more other sniffs from the PHPCS itself that is used in WPCS which cover CSS and JS.

What I found to be the best course of action is to set your ruleset in the project (so in the phpcs.xml.dist)

<arg name="extensions" value="php"/>

And use PHPCS to check only PHP files. For JS and CSS you can use eslint rules and stylelint rules that WordPress uses:

https://www.npmjs.com/package/@wordpress/eslint-plugin/v/1.0.0
https://www.npmjs.com/package/stylelint-config-wordpress

CSS and JS checking in PHPCS will be removed once PHPCS 4.0.0 comes out anyways.

@cagross
Copy link
Author

cagross commented Sep 13, 2019

@dingo-d OK thanks for all that info. I'll use WPCS for PHP, and use the other two rulesets for CSS and JS. In light of that info, I guess we can consider this issue closed? @TonyGravagno, anything objections to me closing this?

@TonyGravagno
Copy link

@dingo-d provided a clear statement about the current state-of-the-art in this area, which I hope will help others. Yes indeed, please close. Thanks again for initiating the discussion.

@jrfnl jrfnl closed this as completed Sep 17, 2019
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

4 participants