Skip to content

Commit

Permalink
Fix @wordpress/comment-case ESlint errors but without adding the di…
Browse files Browse the repository at this point in the history
…sable-rule pragma (#37006)

* Add comment case/punctuation rule

* Add tests for comment-case rule

* Add comment case rule

* Change rule type to problem instead of layout

* Update createFixerFunction to take an error type and handle other rules

* Add messages for missing space and capital letter rule

* Add function to check if comment is on the same line as code

* Check if comment is followed/preceded directly by another comment

This is to determine if it's part of a "multi line" comment.

* Add check to ensure comment token is followed by a space

* Add rule to check for capital letter at the start of comments

* Refactor missingPunctuation rule

* Add test cases for rule

* Modify fixer function to work with block comments

* Handle same-line Block comments and skip translator comments

* Update test cases for new rules

* Remove fixer function

* Update regex to handle translator comments in the same match

* Update tests following removal of fixer function

Since it's been removed, the tests don't need to check the fixer works.

* Add checks for pragmas, URLs, common words, and variable uses

* Ensure standalone comments on consecutive lines are handled

* Add tests for further cases

* Match non-words on trimmed value only

* Add rule to regex to cater for prettier pragma

* Add tests for ignoring prettier pragma in eslint rule

* Allow iPad, iMac, iOS, iPhone to be ignored by eslint rule

* Add comment about what is ignored in commonWordsRegex

* Update pragrma regex and tests to cater for ts-nocheck and globals

* Add noinspection exception to pragma regex

* Add istanbul to pragma exceptions

* Add flow pragmas to exception

* Change comment-case severity from error to warning

* Apply automatic lint fixes for comment-case rule

* Fix falsely flagged lint errors

* Remove period from line where it shouldn't appear

* Suppress lint rule for variable name

testID is referring to a variable, stop it being considered by this rule

* Ignore comment-case rule when referring to a variable

* Ignore comment-case rule when referring to a variable

* Ignore comment-case rule when referring to a variable

* Ignore comment-case rule when referring to a variable

* Ignore comment-case rule when referring to a variable

* Undo changes introduced when prettier-ignore was capitalised

* Ignore comment-case rule when referring to a variable

* Ignore comment-case rule when referring to a variable

* Remove commented out code

* Ignore comment-case rule when referring to a variable

* Remove commented out code

* Reset browserslist-config to lowercase

* Reset ts-expect-error directive

* Reset globals to lowercase

* Remove commented out code and fix incorrectly formatted file

* Reset noinspection to lowercase

* Ignore comment-case rule when referring to a variable

* Reset globals to no period

* Remove falsely fixed lint errors and remove commented out code

* Remove falsely fixed lint errors

* Remove falsely fixed lint errors

* Remove falsely fixed lint errors and remove commented out code

* Remove falsely fixed lint errors

* Undo changes to .eslintrc.js

* Undo changes included in 34964

* Remove references to @wordpress/comment-case rule

* Update missing capitals that the linter skipped

* Remove period from eslint rule disables

* Remove period from @ts-nocheck

* Update snapshot

* Remove periods from end of eslint rule names

* Apply suggestions from code review

Co-authored-by: Greg Ziółkowski <[email protected]>
  • Loading branch information
opr and gziolo authored Feb 25, 2022
1 parent c1df16e commit d591591
Show file tree
Hide file tree
Showing 532 changed files with 2,098 additions and 2,119 deletions.
2 changes: 1 addition & 1 deletion .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ module.exports = {
{
files: [ 'packages/react-native-*/**/*.js' ],
settings: {
'import/ignore': [ 'react-native' ], // Workaround for https://github.com/facebook/react-native/issues/28549
'import/ignore': [ 'react-native' ], // Workaround for https://github.com/facebook/react-native/issues/28549.
},
},
{
Expand Down
1 change: 0 additions & 1 deletion bin/packages/build-worker.js
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,6 @@ async function buildCSS( file ) {
const builtSass = await renderSass( {
file,
includePaths: [ path.join( PACKAGES_DIR, 'base-styles' ) ],
//
data: ''.concat( '@use "sass:math";', importLists, contents ),
} );

Expand Down
4 changes: 2 additions & 2 deletions bin/packages/build.js
Original file line number Diff line number Diff line change
Expand Up @@ -134,10 +134,10 @@ function createStyleEntryTransform() {
entries.forEach( ( entry ) => this.push( entry ) );

// Find other stylesheets that need to be rebuilt because
// they import the styles that are being transformed
// they import the styles that are being transformed.
const styleEntries = findStyleEntriesThatImportFile( file );

// Rebuild stylesheets that import the styles being transformed
// Rebuild stylesheets that import the styles being transformed.
if ( styleEntries.length ) {
styleEntries.forEach( ( entry ) => stream.push( entry ) );
}
Expand Down
10 changes: 5 additions & 5 deletions bin/packages/get-babel-config.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,21 +13,21 @@ module.exports = ( environment = '', file ) => {
};
switch ( environment ) {
case 'main':
// to be merged as a presetEnv option
// To be merged as a presetEnv option.
callerOpts.caller.modules = 'commonjs';
break;
case 'module':
// to be merged as a presetEnv option
// To be merged as a presetEnv option.
callerOpts.caller.modules = false;
// to be merged as a pluginTransformRuntime option
// To be merged as a pluginTransformRuntime option.
callerOpts.caller.useESModules = true;
break;
default:
// preventing measure, this shouldn't happen ever
// Preventing measure, this shouldn't happen ever.
delete callerOpts.caller;
}

// Sourcemaps options
// Sourcemaps options.
const sourceMapsOpts = {
sourceMaps: true,
sourceFileName: file,
Expand Down
2 changes: 1 addition & 1 deletion bin/packages/lint-staged-typecheck.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ const tscPath = path.join( repoRoot, 'node_modules', '.bin', 'tsc' );
// lint-staged passes full paths to staged changes
const changedFiles = process.argv.slice( 2 );

// Transform changed files to package directories containing tsconfig.json
// Transform changed files to package directories containing tsconfig.json.
const changedPackages = _.uniq(
changedFiles.map( ( fullPath ) => {
const relativePath = path.relative( repoRoot, fullPath );
Expand Down
4 changes: 2 additions & 2 deletions bin/plugin/commands/changelog.js
Original file line number Diff line number Diff line change
Expand Up @@ -320,7 +320,7 @@ function getIssueFeature( issue ) {
return rankedFeatures[ 0 ];
}

// 2. `[Feature]` labels
// 2. `[Feature]` labels.
const featureSpecificLabel = getFeatureSpecificLabels( labels );

if ( featureSpecificLabel ) {
Expand Down Expand Up @@ -756,7 +756,7 @@ function getChangelog( pullRequests ) {
function sortFeatureGroups( featureGroups ) {
return Object.keys( featureGroups ).sort(
( featureAName, featureBName ) => {
// Sort "uncategorized" items to *always* be at the top of the section
// Sort "uncategorized" items to *always* be at the top of the section.
if ( featureAName === UNKNOWN_FEATURE_FALLBACK_NAME ) {
return -1;
} else if ( featureBName === UNKNOWN_FEATURE_FALLBACK_NAME ) {
Expand Down
2 changes: 1 addition & 1 deletion bin/plugin/commands/common.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ const config = require( '../config' );
* @return {Promise<string>} Repository local path.
*/
async function runGitRepositoryCloneStep( abortMessage ) {
// Cloning the repository
// Cloning the repository.
let gitWorkingDirectoryPath;
await runStep( 'Cloning the Git repository', abortMessage, async () => {
log( '>> Cloning the Git repository' );
Expand Down
6 changes: 3 additions & 3 deletions bin/plugin/commands/packages.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ async function checkoutNpmReleaseBranch( {
gitWorkingDirectoryPath,
npmReleaseBranch,
} ) {
// Creating the release branch
// Creating the release branch.
await git.checkoutRemoteBranch( gitWorkingDirectoryPath, npmReleaseBranch );
await git.fetch( gitWorkingDirectoryPath, [ '--depth=100' ] );
log(
Expand Down Expand Up @@ -226,7 +226,7 @@ async function updatePackages( config ) {
nextVersion,
version,
} ) => {
// Update changelog
// Update changelog.
const content = await fs.promises.readFile(
changelogPath,
'utf8'
Expand All @@ -243,7 +243,7 @@ async function updatePackages( config ) {
)
);

// Update package.json
// Update package.json.
const packageJson = readJSONFile( packageJSONPath );
const newPackageJson = {
...packageJson,
Expand Down
2 changes: 1 addition & 1 deletion bin/plugin/commands/performance.js
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,7 @@ async function runPerformanceTests( branches, options ) {
results[ testSuite ] = {};
/** @type {Array<Record<string, WPPerformanceResults>>} */
const rawResults = [];
// Alternate three times between branches
// Alternate three times between branches.
for ( let i = 0; i < 3; i++ ) {
rawResults[ i ] = {};
for ( const branch of branches ) {
Expand Down
10 changes: 5 additions & 5 deletions bin/plugin/commands/test/changelog.js
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ describe( 'getIssueFeature', () => {
name: 'Some Label',
},
{
name: '[Package] Example Package', // 1. has explicit mapping
name: '[Package] Example Package', // 1. has explicit mapping.
},
{
name: '[Package] Another One',
Expand All @@ -219,10 +219,10 @@ describe( 'getIssueFeature', () => {
const result = getIssueFeature( {
labels: [
{
name: '[Block] Some Block', // 3. Block-specific label
name: '[Block] Some Block', // 3. Block-specific label.
},
{
name: '[Package] Edit Widgets', // 1. has explicit mapping
name: '[Package] Edit Widgets', // 1. has explicit mapping.
},
{
name: '[Feature] Some Feature', // 2. Feature label.
Expand All @@ -242,13 +242,13 @@ describe( 'getIssueFeature', () => {
const result = getIssueFeature( {
labels: [
{
name: '[Block] Some Block', // block specific label
name: '[Block] Some Block', // Block specific label.
},
{
name: '[Package] This package',
},
{
name: '[Feature] Cool Feature', // should have priority despite prescence of block specific label
name: '[Feature] Cool Feature', // Should have priority despite prescence of block specific label.
},
{
name: '[Package] Another One',
Expand Down
2 changes: 1 addition & 1 deletion bin/plugin/lib/logger.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
*/
const chalk = require( 'chalk' );

// Formats
// Formats.
const title = chalk.bold;
const error = chalk.bold.red;
const warning = chalk.bold.keyword( 'orange' );
Expand Down
1 change: 0 additions & 1 deletion bin/process-git-diff.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
// - "dev": true
// + "dev": true,
// + "optional": true

const hasNonOptionalDiff = !! ( process.argv[ 2 ] || '' )
// Strip individual diffs of optional-only.
.replace( /@@ .+ @@\n(-.+\n\+.+,\n)?\+.+\"optional\": true,?\n/gm, '' )
Expand Down
2 changes: 1 addition & 1 deletion docs/tool/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ const { getRootManifest } = require( './manifest' );
const tocFileInput = path.resolve( __dirname, '../toc.json' );
const manifestOutput = path.resolve( __dirname, '../manifest.json' );

// Process TOC file and generate manifest handbook
// Process TOC file and generate manifest handbook.
fs.writeFileSync(
manifestOutput,
JSON.stringify( getRootManifest( tocFileInput ), undefined, '\t' ).concat(
Expand Down
2 changes: 1 addition & 1 deletion packages/a11y/src/index.native.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import filterMessage from './filter-message';
*/
export function speak( message, ariaLive ) {
message = filterMessage( message );
//TODO: Use native module to speak message
// TODO: Use native module to speak message.
if ( ariaLive === 'assertive' ) {
} else {
}
Expand Down
2 changes: 1 addition & 1 deletion packages/api-fetch/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ const defaultFetchHandler = ( nextOptions ) => {
}

const responsePromise = window.fetch(
// fall back to explicitly passing `window.location` which is the behavior if `undefined` is passed
// Fall back to explicitly passing `window.location` which is the behavior if `undefined` is passed.
url || path || window.location.href,
{
...DEFAULT_OPTIONS,
Expand Down
4 changes: 2 additions & 2 deletions packages/api-fetch/src/middlewares/preloading.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ function createPreloadingMiddleware( preloadedData ) {
if ( 'GET' === method && cache[ path ] ) {
const cacheData = cache[ path ];

// Unsetting the cache key ensures that the data is only used a single time
// Unsetting the cache key ensures that the data is only used a single time.
delete cache[ path ];

return prepareResponse( cacheData, !! parse );
Expand All @@ -50,7 +50,7 @@ function createPreloadingMiddleware( preloadedData ) {
) {
const cacheData = cache[ method ][ path ];

// Unsetting the cache key ensures that the data is only used a single time
// Unsetting the cache key ensures that the data is only used a single time.
delete cache[ method ][ path ];

return prepareResponse( cacheData, !! parse );
Expand Down
20 changes: 10 additions & 10 deletions packages/autop/src/test/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,18 +66,18 @@ test( 'skip pre elements', () => {
done = 0;
});`;

// Not wrapped in <p> tags
// Not wrapped in <p> tags.
let str = `<pre>${ code }</pre>`;
expect( autop( str ).trim() ).toBe( str );

// Text before/after is wrapped in <p> tags
// Text before/after is wrapped in <p> tags.
str = `Look at this code\n\n<pre>${ code }</pre>\n\nIsn't that cool?`;

// Expected text after autop
// Expected text after autop.
let expected = `<p>Look at this code</p>\n<pre>${ code }</pre>\n<p>Isn't that cool?</p>`;
expect( autop( str ).trim() ).toBe( expected );

// Make sure HTML breaks are maintained if manually inserted
// Make sure HTML breaks are maintained if manually inserted.
str =
'Look at this code\n\n<pre>Line1<br />Line2<br>Line3<br/>Line4\nActual Line 2\nActual Line 3</pre>\n\nCool, huh?';
expected =
Expand Down Expand Up @@ -134,7 +134,7 @@ Paragraph two.`;
Paragraph two.`;

const expected =
'<p>Paragraph one.</p>\n' + // line breaks only after <p>
'<p>Paragraph one.</p>\n' + // Line breaks only after <p>
'<p><video class="wp-video-shortcode" id="video-0-1" width="640" height="360" preload="metadata" controls="controls">' +
'<source type="video/mp4" src="http://domain.tld/wp-content/uploads/2013/12/xyz.mp4" />' +
'<!-- WebM/VP8 for Firefox4, Opera, and Chrome -->' +
Expand Down Expand Up @@ -166,7 +166,7 @@ Paragraph two.`;
Paragraph two.`;

const shortcodeExpected =
'<p>Paragraph one.</p>\n' + // line breaks only after <p>
'<p>Paragraph one.</p>\n' + // Line breaks only after <p>
'<p>[video width="720" height="480" mp4="http://domain.tld/wp-content/uploads/2013/12/xyz.mp4"]' +
'<!-- WebM/VP8 for Firefox4, Opera, and Chrome --><source type="video/webm" src="myvideo.webm" />' +
'<!-- Ogg/Vorbis for older Firefox and Opera versions --><source type="video/ogg" src="myvideo.ogv" />' +
Expand Down Expand Up @@ -196,7 +196,7 @@ test( 'param embed elements', () => {
Paragraph two.`;

const expected1 =
'<p>Paragraph one.</p>\n' + // line breaks only after <p>
'<p>Paragraph one.</p>\n' + // Line breaks only after <p>
'<p><object width="400" height="224" classid="clsid:d27cdb6e-ae6d-11cf-96b8-444553540000" codebase="http://download.macromedia.com/pub/shockwave/cabs/flash/swflash.cab#version=6,0,40,0">' +
'<param name="src" value="http://domain.tld/wp-content/uploads/2013/12/xyz.swf" />' +
'<param name="allowfullscreen" value="true" />' +
Expand Down Expand Up @@ -232,7 +232,7 @@ Paragraph two.`;
Paragraph two.`;

const expected2 =
'<p>Paragraph one.</p>\n' + // line breaks only after block tags
'<p>Paragraph one.</p>\n' + // Line breaks only after block tags.
'<div class="video-player" id="x-video-0">\n' +
'<object classid="clsid:D27CDB6E-AE6D-11cf-96B8-444553540000" width="640" height="360" id="video-0" standby="Standby text">' +
'<param name="movie" value="http://domain.tld/wp-content/uploads/2013/12/xyz.swf" />' +
Expand Down Expand Up @@ -318,11 +318,11 @@ test( 'that_autop_treats_block_level_elements_as_blocks', () => {
} );

let expected = content.join( '\n' );
let input = content.join( '\n\n' ); // WS difference
let input = content.join( '\n\n' ); // WS difference.

expect( autop( input ).trim() ).toBe( expected );

input = content.join( '' ); // WS difference
input = content.join( '' ); // WS difference.

expect( autop( input ).trim() ).toBe( expected );

Expand Down
Loading

0 comments on commit d591591

Please sign in to comment.