Skip to content

Update sniff to remove empty new lines at the start of if conditions#2773

Merged
Crabcyborg merged 1 commit into
masterfrom
update_sniff_to_remove_empty_new_lines_at_the_start_of_if_conditions
Jan 12, 2026
Merged

Update sniff to remove empty new lines at the start of if conditions#2773
Crabcyborg merged 1 commit into
masterfrom
update_sniff_to_remove_empty_new_lines_at_the_start_of_if_conditions

Conversation

@Crabcyborg
Copy link
Copy Markdown
Contributor

No description provided.

@Crabcyborg Crabcyborg added this to the 6.27 milestone Jan 12, 2026
@Crabcyborg Crabcyborg merged commit 5a77051 into master Jan 12, 2026
37 of 38 checks passed
@Crabcyborg Crabcyborg deleted the update_sniff_to_remove_empty_new_lines_at_the_start_of_if_conditions branch January 12, 2026 13:39
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Jan 12, 2026

Walkthrough

A new PHP_CodeSniffer sniff enforces removal of blank lines after if/elseif/else opening braces, with the formatting applied across twelve existing files to comply with this code standard.

Changes

Cohort / File(s) Summary
Standards Enforcement Sniff
phpcs-sniffs/Formidable/Sniffs/WhiteSpace/NoBlankLineAfterIfOpenSniff.php
New sniff class that detects and fixes blank lines immediately after if/elseif/else block opening braces. Registers tokens T_IF, T_ELSEIF, T_ELSE and validates that first non-whitespace content appears on same or next line; collapses excessive newlines to single newline with proper indentation.
Controllers
classes/controllers/FrmFormsController.php
Blank line removed after else block in get_form_contents.
Helpers
classes/helpers/FrmXMLHelper.php
Three blank lines removed: in import_xml_fields, populate_postmeta, and migrate_email_settings_to_action.
Models — Entry & Form Data
classes/models/FrmEntryMeta.php, classes/models/FrmEntryFormatter.php, classes/models/FrmForm.php
Blank lines removed after conditional blocks in update_entry_metas, push_single_field_to_array, and maybe_update_max_option.
Models — Email & Notifications
classes/models/FrmEmail.php, classes/models/FrmAddon.php, classes/models/FrmNotification.php
Blank lines removed in format_from, edd_plugin_updater, and print_recipients.
Models — File, Honeypot & HTML
classes/models/FrmCreateFile.php, classes/models/FrmHoneypot.php, classes/models/FrmTableHTMLGenerator.php
Blank lines removed in append_file, is_honeypot_spam, and init_table_style/init_td_style.
Views
classes/views/frm-fields/back-end/radio-field.php
Blank line removed between elseif and foreach loop.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Description check ❓ Inconclusive No pull request description was provided by the author, making it impossible to assess relevance to the changeset. Add a pull request description explaining the purpose of the new sniff and why these formatting changes are necessary.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the main change: a new PHP_CodeSniffer sniff that detects and removes blank lines after if/elseif/else opening braces, with corresponding formatting updates across the codebase.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c94f880 and 406fd38.

⛔ Files ignored due to path filters (1)
  • phpcs.xml is excluded by !**/*.xml
📒 Files selected for processing (13)
  • classes/controllers/FrmFormsController.php
  • classes/helpers/FrmXMLHelper.php
  • classes/models/FrmAddon.php
  • classes/models/FrmCreateFile.php
  • classes/models/FrmEmail.php
  • classes/models/FrmEntryFormatter.php
  • classes/models/FrmEntryMeta.php
  • classes/models/FrmForm.php
  • classes/models/FrmHoneypot.php
  • classes/models/FrmNotification.php
  • classes/models/FrmTableHTMLGenerator.php
  • classes/views/frm-fields/back-end/radio-field.php
  • phpcs-sniffs/Formidable/Sniffs/WhiteSpace/NoBlankLineAfterIfOpenSniff.php
💤 Files with no reviewable changes (12)
  • classes/controllers/FrmFormsController.php
  • classes/models/FrmTableHTMLGenerator.php
  • classes/views/frm-fields/back-end/radio-field.php
  • classes/models/FrmAddon.php
  • classes/models/FrmNotification.php
  • classes/models/FrmForm.php
  • classes/models/FrmHoneypot.php
  • classes/models/FrmEntryFormatter.php
  • classes/models/FrmEntryMeta.php
  • classes/models/FrmEmail.php
  • classes/helpers/FrmXMLHelper.php
  • classes/models/FrmCreateFile.php
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
  • GitHub Check: PHP 8 tests in WP 6.9
  • GitHub Check: PHP 7.4 tests in WP 6.9
  • GitHub Check: Cypress
  • GitHub Check: PHP 7.4 tests in WP 6.9
  • GitHub Check: Cypress
  • GitHub Check: PHP 8 tests in WP 6.9
  • GitHub Check: PHP 8 tests in WP 6.9
  • GitHub Check: PHP 7.4 tests in WP 6.9
  • GitHub Check: Cypress
🔇 Additional comments (1)
phpcs-sniffs/Formidable/Sniffs/WhiteSpace/NoBlankLineAfterIfOpenSniff.php (1)

48-75: LGTM! Detection logic is correct.

The scope check properly handles alternative syntax (without braces), empty block handling is appropriate, and the line comparison logic correctly identifies blank lines (content more than 1 line after opener).

Comment on lines +98 to +107
} elseif ( $newlineCount === 1 ) {
// Single newline here, but there might be another whitespace token creating the blank.
// Check if the next token is also whitespace with a newline.
$afterNext = $nextToken + 1;

if ( $afterNext < $firstContent && $tokens[ $afterNext ]['code'] === T_WHITESPACE ) {
// Remove this extra whitespace token.
$phpcsFile->fixer->replaceToken( $afterNext, '' );
}
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Potential indentation loss when fixing multi-token whitespace.

If the whitespace is split across multiple tokens (e.g., first token contains "\n" and second token contains "\n "), removing the second token entirely at line 105 will lose the indentation. The content would appear flush-left after the fix.

Consider preserving indentation by extracting it from the token closest to $firstContent:

Suggested fix
 			} elseif ( $newlineCount === 1 ) {
 				// Single newline here, but there might be another whitespace token creating the blank.
-				// Check if the next token is also whitespace with a newline.
-				$afterNext = $nextToken + 1;
-
-				if ( $afterNext < $firstContent && $tokens[ $afterNext ]['code'] === T_WHITESPACE ) {
-					// Remove this extra whitespace token.
-					$phpcsFile->fixer->replaceToken( $afterNext, '' );
+				// Find the token right before content to get proper indentation.
+				$beforeContent = $firstContent - 1;
+				if ( $tokens[ $beforeContent ]['code'] === T_WHITESPACE ) {
+					$wsContent   = $tokens[ $beforeContent ]['content'];
+					$lastNewline = strrpos( $wsContent, "\n" );
+					$indent      = ( false !== $lastNewline ) ? substr( $wsContent, $lastNewline + 1 ) : $wsContent;
+
+					// Replace first whitespace with single newline + proper indent.
+					$phpcsFile->fixer->replaceToken( $nextToken, "\n" . $indent );
+
+					// Remove all intermediate whitespace tokens.
+					for ( $i = $nextToken + 1; $i < $firstContent; $i++ ) {
+						if ( $tokens[ $i ]['code'] === T_WHITESPACE ) {
+							$phpcsFile->fixer->replaceToken( $i, '' );
+						}
+					}
 				}
 			}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
} elseif ( $newlineCount === 1 ) {
// Single newline here, but there might be another whitespace token creating the blank.
// Check if the next token is also whitespace with a newline.
$afterNext = $nextToken + 1;
if ( $afterNext < $firstContent && $tokens[ $afterNext ]['code'] === T_WHITESPACE ) {
// Remove this extra whitespace token.
$phpcsFile->fixer->replaceToken( $afterNext, '' );
}
}
} elseif ( $newlineCount === 1 ) {
// Single newline here, but there might be another whitespace token creating the blank.
// Find the token right before content to get proper indentation.
$beforeContent = $firstContent - 1;
if ( $tokens[ $beforeContent ]['code'] === T_WHITESPACE ) {
$wsContent = $tokens[ $beforeContent ]['content'];
$lastNewline = strrpos( $wsContent, "\n" );
$indent = ( false !== $lastNewline ) ? substr( $wsContent, $lastNewline + 1 ) : $wsContent;
// Replace first whitespace with single newline + proper indent.
$phpcsFile->fixer->replaceToken( $nextToken, "\n" . $indent );
// Remove all intermediate whitespace tokens.
for ( $i = $nextToken + 1; $i < $firstContent; $i++ ) {
if ( $tokens[ $i ]['code'] === T_WHITESPACE ) {
$phpcsFile->fixer->replaceToken( $i, '' );
}
}
}
}

stephywells pushed a commit that referenced this pull request Apr 4, 2026
…_new_lines_at_the_start_of_if_conditions

Update sniff to remove empty new lines at the start of if conditions
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant