No Auto Generated Themes #65

khacoder · 2016-08-19T22:48:51Z

Issue #63
Verify that themes are not generated, by searching for strings contained in known auto generated packages.

grappler · 2016-09-18T13:09:40Z

This looks good. It will need an update to pass the automated tests.

grappler · 2016-10-01T15:48:44Z

I have rebased the branch so that it is up to date with the latest code and this get's travis to run it's latest checks. To update the branch locally run git reset origin/feature/no-auto-generate --hard

grappler · 2016-10-17T02:17:21Z

The PR is ready for the final review. I have simplified the checks to only check for the names. When downloading the latest version of the generated themes I did not find the code that the check was looking for. These are not common words so there should not be any problem with false possitives.

jrfnl · 2016-10-11T16:35:41Z

WordPress/Tests/Theme/NoAutoGenerateUnitTest.php

+ * @category PHP
+ * @package  PHP_CodeSniffer
+ * @link     https://make.wordpress.org/core/handbook/best-practices/coding-standards/
+ */


Please update the doc block to comply with the current standards.

jrfnl · 2016-10-22T17:22:09Z

WordPress/Sniffs/Theme/NoAutoGenerateSniff.php

+			return;
+		}
+
+		$generated_themes = array(


This should be a class property, similar to the include/require sniff class property.

jrfnl · 2016-10-22T17:24:42Z

WordPress/Sniffs/Theme/NoAutoGenerateSniff.php

+			'wpthemegenerator',
+		);
+
+		foreach ( $generated_themes as $generated_theme ) {


Similar to the favicon one, we could probably use a regex here, though as it's only four strings being sniffed for, it's not a breaking issue at the moment.
If/when the list gets longer, I would very much suggest changing it to a regex.

jrfnl · 2016-10-22T17:26:14Z

WordPress/Sniffs/Theme/NoAutoGenerateSniff.php

+
+		foreach ( $generated_themes as $generated_theme ) {
+			if ( false !== strpos( strtolower( $token['content'] ), $generated_theme ) ) {
+				$phpcsFile->addError( 'Auto generated themes are not allowed in the theme directory.', $stackPtr, 'AutoGeneratedFound' );


Maybe add a Found: %s to the error sentence ? Or is this left out on purpose ?

jrfnl · 2016-10-22T17:27:18Z

WordPress/Tests/Theme/NoAutoGenerateUnitTest.inc

+// wpthemegenerator.com
+/*
+ * @package templatetoaster
+ */


As you are sniffing for both PHP as well as CSS, you will need a css example file too and deal with that in the unit test file.

jrfnl · 2016-10-22T17:29:40Z

WordPress/Sniffs/Theme/NoAutoGenerateSniff.php

+			// TemplateToaster.
+			'templatetoaster',
+			// WP Theme Generator.
+			'wpthemegenerator',


Looks like the list has become a lot shorter than it originally was. Should the other snippets no longer be sniffed for ? How will they be dealt with ?

I mentioned in the comment above:

When downloading the latest version of the generated themes I did not find the code that the check was looking for.

Ah, I misread that comment before.

Still, serial theme builders often don't download a new version of base code for each theme (take TGMPA for example... sigh...), so should we may be still check for code from at least a few versions back ?

I don't think this should be a big problem any more. This was a bigger problem a few years ago. As we have been blocking the generators spammers should know by no they are not allowed.

You and the other members of the TRT are the better judge of that. I just wonder if relaxing the sniffs will not make some of them think "hang on, now we have a chance again...."

jrfnl · 2016-10-22T17:32:44Z

WordPress/Tests/Theme/NoAutoGenerateUnitTest.inc

@@ -0,0 +1,11 @@
+
+<p>Lubith</p>
+


Are these "real life" examples or made up examples ? I would very much prefer to see real life examples here to demonstrate what is being sniffed for and how.

Also, let's add some examples to make sure we don't get false positives.

grappler · 2016-12-18T20:11:51Z

We need to add https://pinegrow.com/wordpress-theme-builder.html to the list.
Example of theme build with it: https://themes.svn.wordpress.org/dokumentasi-template/1.0/

grappler · 2016-12-18T23:57:05Z

@jrfnl I have made the requested changes. Thank you for pushing me use real world examples. Found a few bugs that way. Once I get the greenlight I can squash the commits.

jrfnl

Looking good!
Two points remaining:

the file doc block at the top of the unit test file (see previous review)
adding some test data to make sure we won't get false positives.

grappler · 2016-12-19T12:50:29Z

I have updated file doc block. Do you have any ideas for test data to make sure we do not get false positives. I can't think of any. The keywords are unique enough that normal users should not be using them.

The only idea that I had was to split the keywords up so that they were normal words. e.g. artist, them, template, toaster, pine, grow, theme, wp

jrfnl · 2016-12-19T20:11:21Z

Do you have any ideas for test data to make sure we do not get false positives. I can't think of any. The keywords are unique enough that normal users should not be using them.

Just some random thoughts on this:

CSS - add a header or two or ok themes (twentyseventeen and a non-automattic ok theme)
Add some lines containing things like: artisteers, wpthemler, glubith

Does that help ?

jrfnl · 2016-12-19T22:28:17Z

WordPress/Tests/Theme/NoAutoGenerateUnitTest.php

+					2  => 1,
+					3  => 1,
+					6  => 1,
+					6  => 1,


Looks like there is a duplicate line here

Issue #63 Verify that themes are not generated, by searching for strings contained in known auto generated packages.

grappler · 2016-12-19T22:32:00Z

Fixed the last bug. There is a chance for false positives if a theme uses any of the restricted words but the chances and reasons to do so it is very low.

grappler added the pr: needs review label Sep 27, 2016

grappler force-pushed the feature/theme-review-sniffs branch 2 times, most recently from 563cd1d to 6671823 Compare September 27, 2016 12:28

jrfnl added pr: needs update pr: needs review and removed pr: needs review pr: needs update labels Oct 22, 2016

jrfnl requested changes Oct 22, 2016

View reviewed changes

jrfnl added pr: needs update and removed pr: needs review labels Oct 22, 2016

grappler mentioned this pull request Dec 18, 2016

Add sniffs for the theme checks WordPress/WordPress-Coding-Standards#578

Closed

72 tasks

grappler added pr: needs review and removed pr: needs update labels Dec 18, 2016

jrfnl requested changes Dec 19, 2016

View reviewed changes

jrfnl reviewed Dec 19, 2016

View reviewed changes

WordPress/Tests/Theme/NoAutoGenerateUnitTest.php

2 => 1,

3 => 1,

6 => 1,

6 => 1,

Copy link

jrfnl Dec 19, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like there is a duplicate line here

No Auto Generated Themes

6f693ad

Issue #63 Verify that themes are not generated, by searching for strings contained in known auto generated packages.

jrfnl approved these changes Dec 19, 2016

View reviewed changes

jrfnl merged commit 13e9146 into WPTT:feature/theme-review-sniffs Dec 19, 2016

grappler deleted the feature/no-auto-generate branch December 19, 2016 22:50

jrfnl added this to the 0.1.0 milestone Aug 20, 2018

jrfnl removed the pr: needs review label Oct 14, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

No Auto Generated Themes #65

No Auto Generated Themes #65

khacoder commented Aug 19, 2016

grappler commented Sep 18, 2016

grappler commented Oct 1, 2016

grappler commented Oct 17, 2016

jrfnl Oct 11, 2016

jrfnl Oct 22, 2016

jrfnl Oct 22, 2016

jrfnl Oct 22, 2016

jrfnl Oct 22, 2016

jrfnl Oct 22, 2016

grappler Oct 23, 2016

jrfnl Oct 24, 2016

grappler Oct 24, 2016

jrfnl Oct 24, 2016

jrfnl Oct 22, 2016

grappler commented Dec 18, 2016

grappler commented Dec 18, 2016

jrfnl left a comment

grappler commented Dec 19, 2016

jrfnl commented Dec 19, 2016

jrfnl Dec 19, 2016

grappler commented Dec 19, 2016

+=> 1,
+=> 1,
+=> 1,
+=> 1,

No Auto Generated Themes #65

No Auto Generated Themes #65

Conversation

khacoder commented Aug 19, 2016

grappler commented Sep 18, 2016

grappler commented Oct 1, 2016

grappler commented Oct 17, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

grappler commented Dec 18, 2016

grappler commented Dec 18, 2016

jrfnl left a comment

Choose a reason for hiding this comment

grappler commented Dec 19, 2016

jrfnl commented Dec 19, 2016

Choose a reason for hiding this comment

grappler commented Dec 19, 2016