From 43539b220ce28869073422a772ad72c793a7d310 Mon Sep 17 00:00:00 2001 From: Andre Klapper Date: Sat, 24 Aug 2024 10:46:56 +0200 Subject: [PATCH] Remove trivial cases of unreachable code Summary: Static code analysis can detect `Unreachable statement - code above always terminates.` The vast majority of occurrences in the Phorge codebase are due to an unreachable `break` within a `case` after a `return` or after an all-covering `if/else`. All this noise makes it harder to spot real logic issues (there are some!), thus fix these trivial cases. Test Plan: Examine the code; run static code analysis. Reviewers: O1 Blessed Committers, valerio.bozzolan Reviewed By: O1 Blessed Committers, valerio.bozzolan Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno Differential Revision: https://we.phorge.it/D25802 --- src/applications/auth/factor/PhabricatorDuoAuthFactor.php | 1 - .../storage/PhabricatorAuthProviderConfigTransaction.php | 8 -------- .../calendar/parser/data/PhutilCalendarRecurrenceRule.php | 1 - .../config/storage/PhabricatorConfigTransaction.php | 3 --- .../conduit/ConpherenceCreateThreadConduitAPIMethod.php | 1 - .../controller/ConpherenceUpdateController.php | 4 ---- .../conpherence/storage/ConpherenceThread.php | 1 - .../conpherence/view/ConpherenceTransactionView.php | 1 - .../conduit/DifferentialCreateCommentConduitAPIMethod.php | 1 - .../differential/storage/DifferentialTransaction.php | 3 --- .../diffusion/conduit/DiffusionQueryConduitAPIMethod.php | 1 - .../diffusion/engine/DiffusionCommitHookEngine.php | 1 - src/applications/feed/story/PhabricatorFeedStory.php | 1 - .../PhabricatorFilesManagementRebuildWorkflow.php | 1 - src/applications/files/storage/PhabricatorFile.php | 3 --- src/applications/flag/query/PhabricatorFlagQuery.php | 1 - src/applications/herald/field/HeraldField.php | 1 - .../legalpad/storage/LegalpadDocumentBody.php | 1 - .../maniphest/storage/ManiphestTransaction.php | 1 - src/applications/nuance/github/NuanceGitHubRawEvent.php | 5 ----- src/applications/phame/storage/PhamePost.php | 1 - src/applications/phrequent/storage/PhrequentTimeBlock.php | 1 - .../setting/PhabricatorConpherenceSoundSetting.php | 3 --- .../PhabricatorApplicationTransactionValueController.php | 1 - .../storage/PhabricatorApplicationTransaction.php | 8 -------- src/view/phui/PHUIHeaderView.php | 1 - 26 files changed, 55 deletions(-) diff --git a/src/applications/auth/factor/PhabricatorDuoAuthFactor.php b/src/applications/auth/factor/PhabricatorDuoAuthFactor.php index 65f0aa5e4b..32a248b372 100644 --- a/src/applications/auth/factor/PhabricatorDuoAuthFactor.php +++ b/src/applications/auth/factor/PhabricatorDuoAuthFactor.php @@ -650,7 +650,6 @@ final class PhabricatorDuoAuthFactor 'You denied this request. Wait %s second(s) to try again.', new PhutilNumber($wait_duration))); } - break; } return null; diff --git a/src/applications/auth/storage/PhabricatorAuthProviderConfigTransaction.php b/src/applications/auth/storage/PhabricatorAuthProviderConfigTransaction.php index 92bb4883bc..39f9fdbc25 100644 --- a/src/applications/auth/storage/PhabricatorAuthProviderConfigTransaction.php +++ b/src/applications/auth/storage/PhabricatorAuthProviderConfigTransaction.php @@ -79,7 +79,6 @@ final class PhabricatorAuthProviderConfigTransaction '%s disabled this provider.', $this->renderHandleLink($author_phid)); } - break; case self::TYPE_LOGIN: if ($new) { return pht( @@ -90,7 +89,6 @@ final class PhabricatorAuthProviderConfigTransaction '%s disabled login.', $this->renderHandleLink($author_phid)); } - break; case self::TYPE_REGISTRATION: if ($new) { return pht( @@ -101,7 +99,6 @@ final class PhabricatorAuthProviderConfigTransaction '%s disabled registration.', $this->renderHandleLink($author_phid)); } - break; case self::TYPE_LINK: if ($new) { return pht( @@ -112,7 +109,6 @@ final class PhabricatorAuthProviderConfigTransaction '%s disabled account linking.', $this->renderHandleLink($author_phid)); } - break; case self::TYPE_UNLINK: if ($new) { return pht( @@ -123,7 +119,6 @@ final class PhabricatorAuthProviderConfigTransaction '%s disabled account unlinking.', $this->renderHandleLink($author_phid)); } - break; case self::TYPE_TRUST_EMAILS: if ($new) { return pht( @@ -134,7 +129,6 @@ final class PhabricatorAuthProviderConfigTransaction '%s disabled email trust.', $this->renderHandleLink($author_phid)); } - break; case self::TYPE_AUTO_LOGIN: if ($new) { return pht( @@ -145,7 +139,6 @@ final class PhabricatorAuthProviderConfigTransaction '%s disabled auto login.', $this->renderHandleLink($author_phid)); } - break; case self::TYPE_PROPERTY: $provider = $this->getProvider(); if ($provider) { @@ -158,7 +151,6 @@ final class PhabricatorAuthProviderConfigTransaction return pht( '%s edited a property of this provider.', $this->renderHandleLink($author_phid)); - break; } return parent::getTitle(); diff --git a/src/applications/calendar/parser/data/PhutilCalendarRecurrenceRule.php b/src/applications/calendar/parser/data/PhutilCalendarRecurrenceRule.php index 504f7d8e9e..6422b59e16 100644 --- a/src/applications/calendar/parser/data/PhutilCalendarRecurrenceRule.php +++ b/src/applications/calendar/parser/data/PhutilCalendarRecurrenceRule.php @@ -575,7 +575,6 @@ final class PhutilCalendarRecurrenceRule pht( 'RRULE specifies BYMONTHDAY with FREQ set to WEEKLY, which '. 'violates RFC5545.')); - break; default: break; } diff --git a/src/applications/config/storage/PhabricatorConfigTransaction.php b/src/applications/config/storage/PhabricatorConfigTransaction.php index 94272bfb1a..b89ae7d838 100644 --- a/src/applications/config/storage/PhabricatorConfigTransaction.php +++ b/src/applications/config/storage/PhabricatorConfigTransaction.php @@ -45,7 +45,6 @@ final class PhabricatorConfigTransaction '%s edited this configuration entry.', $this->renderHandleLink($author_phid)); } - break; } return parent::getTitle(); @@ -83,7 +82,6 @@ final class PhabricatorConfigTransaction $this->renderHandleLink($author_phid), $this->getObject()->getConfigKey()); } - break; } return parent::getTitle(); @@ -145,7 +143,6 @@ final class PhabricatorConfigTransaction } else { return PhabricatorTransactions::COLOR_BLUE; } - break; } } diff --git a/src/applications/conpherence/conduit/ConpherenceCreateThreadConduitAPIMethod.php b/src/applications/conpherence/conduit/ConpherenceCreateThreadConduitAPIMethod.php index e751318c16..8ff324bbb6 100644 --- a/src/applications/conpherence/conduit/ConpherenceCreateThreadConduitAPIMethod.php +++ b/src/applications/conpherence/conduit/ConpherenceCreateThreadConduitAPIMethod.php @@ -62,7 +62,6 @@ final class ConpherenceCreateThreadConduitAPIMethod switch ($error_code) { case ConpherenceEditor::ERROR_EMPTY_PARTICIPANTS: throw new ConduitException('ERR_EMPTY_PARTICIPANT_PHIDS'); - break; } } } diff --git a/src/applications/conpherence/controller/ConpherenceUpdateController.php b/src/applications/conpherence/controller/ConpherenceUpdateController.php index 1b2ae3ab62..cbac0779db 100644 --- a/src/applications/conpherence/controller/ConpherenceUpdateController.php +++ b/src/applications/conpherence/controller/ConpherenceUpdateController.php @@ -139,7 +139,6 @@ final class ConpherenceUpdateController break; default: throw new Exception(pht('Unknown action: %s', $action)); - break; } if ($xactions) { @@ -173,19 +172,16 @@ final class ConpherenceUpdateController $latest_transaction_id); return id(new AphrontAjaxResponse()) ->setContent($content); - break; case 'go-home': $content = array( 'href' => $this->getApplicationURI(), ); return id(new AphrontAjaxResponse()) ->setContent($content); - break; case 'redirect': default: return id(new AphrontRedirectResponse()) ->setURI('/'.$conpherence->getMonogram()); - break; } } } diff --git a/src/applications/conpherence/storage/ConpherenceThread.php b/src/applications/conpherence/storage/ConpherenceThread.php index 7a5f97ed41..a2150662c6 100644 --- a/src/applications/conpherence/storage/ConpherenceThread.php +++ b/src/applications/conpherence/storage/ConpherenceThread.php @@ -271,7 +271,6 @@ final class ConpherenceThread extends ConpherenceDAO switch ($capability) { case PhabricatorPolicyCapability::CAN_VIEW: return pht('Participants in a room can always view it.'); - break; } } diff --git a/src/applications/conpherence/view/ConpherenceTransactionView.php b/src/applications/conpherence/view/ConpherenceTransactionView.php index d976fb604f..cf054133cd 100644 --- a/src/applications/conpherence/view/ConpherenceTransactionView.php +++ b/src/applications/conpherence/view/ConpherenceTransactionView.php @@ -88,7 +88,6 @@ final class ConpherenceTransactionView extends AphrontView { $viewer, 'M jS, Y')), )); - break; } $info = $this->renderTransactionInfo(); diff --git a/src/applications/differential/conduit/DifferentialCreateCommentConduitAPIMethod.php b/src/applications/differential/conduit/DifferentialCreateCommentConduitAPIMethod.php index e85c0db198..c179c9fa70 100644 --- a/src/applications/differential/conduit/DifferentialCreateCommentConduitAPIMethod.php +++ b/src/applications/differential/conduit/DifferentialCreateCommentConduitAPIMethod.php @@ -81,7 +81,6 @@ final class DifferentialCreateCommentConduitAPIMethod pht( 'Unsupported action "%s".', $action)); - break; } } diff --git a/src/applications/differential/storage/DifferentialTransaction.php b/src/applications/differential/storage/DifferentialTransaction.php index d60fdc4bbd..54ece849d0 100644 --- a/src/applications/differential/storage/DifferentialTransaction.php +++ b/src/applications/differential/storage/DifferentialTransaction.php @@ -252,11 +252,9 @@ final class DifferentialTransaction $commit_name, $author_name); } - break; default: return DifferentialAction::getBasicStoryText($new, $author_handle); } - break; } return parent::getTitle(); @@ -366,7 +364,6 @@ final class DifferentialTransaction $commit_name); } } - break; case DifferentialAction::ACTION_REQUEST: return pht( diff --git a/src/applications/diffusion/conduit/DiffusionQueryConduitAPIMethod.php b/src/applications/diffusion/conduit/DiffusionQueryConduitAPIMethod.php index cd6a45360b..d797635985 100644 --- a/src/applications/diffusion/conduit/DiffusionQueryConduitAPIMethod.php +++ b/src/applications/diffusion/conduit/DiffusionQueryConduitAPIMethod.php @@ -162,7 +162,6 @@ abstract class DiffusionQueryConduitAPIMethod break; default: throw new ConduitException('ERR-UNKNOWN-VCS-TYPE'); - break; } return $result; } diff --git a/src/applications/diffusion/engine/DiffusionCommitHookEngine.php b/src/applications/diffusion/engine/DiffusionCommitHookEngine.php index e4e8fd2293..27f7410090 100644 --- a/src/applications/diffusion/engine/DiffusionCommitHookEngine.php +++ b/src/applications/diffusion/engine/DiffusionCommitHookEngine.php @@ -1371,7 +1371,6 @@ final class DiffusionCommitHookEngine extends Phobject { return id(new DiffusionCommitRef()) ->setMessage($message); - break; default: throw new Exception(pht("Unknown VCS '%s!'", $vcs)); } diff --git a/src/applications/feed/story/PhabricatorFeedStory.php b/src/applications/feed/story/PhabricatorFeedStory.php index 2d0ed8836b..9ef05bddea 100644 --- a/src/applications/feed/story/PhabricatorFeedStory.php +++ b/src/applications/feed/story/PhabricatorFeedStory.php @@ -223,7 +223,6 @@ abstract class PhabricatorFeedStory break; default: throw new Exception(pht('Unknown rendering target: %s', $target)); - break; } } diff --git a/src/applications/files/management/PhabricatorFilesManagementRebuildWorkflow.php b/src/applications/files/management/PhabricatorFilesManagementRebuildWorkflow.php index e577a04d55..4e6dd4302a 100644 --- a/src/applications/files/management/PhabricatorFilesManagementRebuildWorkflow.php +++ b/src/applications/files/management/PhabricatorFilesManagementRebuildWorkflow.php @@ -139,6 +139,5 @@ final class PhabricatorFilesManagementRebuildWorkflow return 0; } - return 0; } } diff --git a/src/applications/files/storage/PhabricatorFile.php b/src/applications/files/storage/PhabricatorFile.php index 16681973e0..3da430d95c 100644 --- a/src/applications/files/storage/PhabricatorFile.php +++ b/src/applications/files/storage/PhabricatorFile.php @@ -1038,13 +1038,10 @@ final class PhabricatorFile extends PhabricatorFileDAO case 'jpg'; case 'jpeg': return function_exists('imagejpeg'); - break; case 'png': return function_exists('imagepng'); - break; case 'gif': return function_exists('imagegif'); - break; default: throw new Exception(pht('Unknown type matched as image MIME type.')); } diff --git a/src/applications/flag/query/PhabricatorFlagQuery.php b/src/applications/flag/query/PhabricatorFlagQuery.php index 07b3e09d10..f96e64ea81 100644 --- a/src/applications/flag/query/PhabricatorFlagQuery.php +++ b/src/applications/flag/query/PhabricatorFlagQuery.php @@ -123,7 +123,6 @@ final class PhabricatorFlagQuery default: throw new Exception( pht('Unknown groupBy parameter: %s', $this->groupBy)); - break; } return $flags; diff --git a/src/applications/herald/field/HeraldField.php b/src/applications/herald/field/HeraldField.php index cdfdd7e518..f4cc9fd442 100644 --- a/src/applications/herald/field/HeraldField.php +++ b/src/applications/herald/field/HeraldField.php @@ -144,7 +144,6 @@ abstract class HeraldField extends Phobject { return $tokenizer; } - break; } diff --git a/src/applications/legalpad/storage/LegalpadDocumentBody.php b/src/applications/legalpad/storage/LegalpadDocumentBody.php index 001e38ebf3..82809b201a 100644 --- a/src/applications/legalpad/storage/LegalpadDocumentBody.php +++ b/src/applications/legalpad/storage/LegalpadDocumentBody.php @@ -54,7 +54,6 @@ final class LegalpadDocumentBody extends LegalpadDAO break; default: throw new Exception(pht('Unknown field: %s', $field)); - break; } return $text; diff --git a/src/applications/maniphest/storage/ManiphestTransaction.php b/src/applications/maniphest/storage/ManiphestTransaction.php index 297bb970d1..54a1e47f6c 100644 --- a/src/applications/maniphest/storage/ManiphestTransaction.php +++ b/src/applications/maniphest/storage/ManiphestTransaction.php @@ -130,7 +130,6 @@ final class ManiphestTransaction $this->renderHandleLink($author_phid), $this->renderSubtypeName($old), $this->renderSubtypeName($new)); - break; } return parent::getTitle(); diff --git a/src/applications/nuance/github/NuanceGitHubRawEvent.php b/src/applications/nuance/github/NuanceGitHubRawEvent.php index 8652dca9ad..d203bd52b9 100644 --- a/src/applications/nuance/github/NuanceGitHubRawEvent.php +++ b/src/applications/nuance/github/NuanceGitHubRawEvent.php @@ -266,7 +266,6 @@ final class NuanceGitHubRawEvent extends Phobject { default: return pht('Ref %s', $ref); } - break; case 'PushEvent': $ref = idxv($raw, array('payload', 'ref')); if (preg_match('(^refs/heads/)', $ref)) { @@ -274,7 +273,6 @@ final class NuanceGitHubRawEvent extends Phobject { } else { return pht('Ref %s', $ref); } - break; case 'WatchEvent': $actor = idxv($raw, array('actor', 'login')); return pht('User %s', $actor); @@ -362,7 +360,6 @@ final class NuanceGitHubRawEvent extends Phobject { default: return pht('"%s"', $action); } - break; case 'IssueCommentEvent': $action = idxv($raw, array('payload', 'action')); switch ($action) { @@ -371,7 +368,6 @@ final class NuanceGitHubRawEvent extends Phobject { default: return pht('"%s"', $action); } - break; case 'PullRequestEvent': $action = idxv($raw, array('payload', 'action')); switch ($action) { @@ -380,7 +376,6 @@ final class NuanceGitHubRawEvent extends Phobject { default: return pht('"%s"', $action); } - break; case 'WatchEvent': return pht('Watched'); } diff --git a/src/applications/phame/storage/PhamePost.php b/src/applications/phame/storage/PhamePost.php index 9b4a6ba01a..a7d3129f81 100644 --- a/src/applications/phame/storage/PhamePost.php +++ b/src/applications/phame/storage/PhamePost.php @@ -222,7 +222,6 @@ final class PhamePost extends PhameDAO } else { return PhabricatorPolicies::POLICY_NOONE; } - break; case PhabricatorPolicyCapability::CAN_EDIT: if ($this->getBlog()) { return $this->getBlog()->getEditPolicy(); diff --git a/src/applications/phrequent/storage/PhrequentTimeBlock.php b/src/applications/phrequent/storage/PhrequentTimeBlock.php index 8649b4f67a..5051c050d7 100644 --- a/src/applications/phrequent/storage/PhrequentTimeBlock.php +++ b/src/applications/phrequent/storage/PhrequentTimeBlock.php @@ -318,7 +318,6 @@ final class PhrequentTimeBlock extends Phobject { } } - return 0; } } diff --git a/src/applications/settings/setting/PhabricatorConpherenceSoundSetting.php b/src/applications/settings/setting/PhabricatorConpherenceSoundSetting.php index 77ff6eac46..e5ec008f77 100644 --- a/src/applications/settings/setting/PhabricatorConpherenceSoundSetting.php +++ b/src/applications/settings/setting/PhabricatorConpherenceSoundSetting.php @@ -50,7 +50,6 @@ final class PhabricatorConpherenceSoundSetting ConpherenceRoomSettings::SOUND_MENTION => ConpherenceRoomSettings::DEFAULT_MENTION_SOUND, ); - break; case self::VALUE_CONPHERENCE_MENTION: return array( ConpherenceRoomSettings::SOUND_RECEIVE => @@ -58,7 +57,6 @@ final class PhabricatorConpherenceSoundSetting ConpherenceRoomSettings::SOUND_MENTION => ConpherenceRoomSettings::DEFAULT_MENTION_SOUND, ); - break; case self::VALUE_CONPHERENCE_SILENT: return array( ConpherenceRoomSettings::SOUND_RECEIVE => @@ -66,7 +64,6 @@ final class PhabricatorConpherenceSoundSetting ConpherenceRoomSettings::SOUND_MENTION => ConpherenceRoomSettings::DEFAULT_NO_SOUND, ); - break; } } diff --git a/src/applications/transactions/controller/PhabricatorApplicationTransactionValueController.php b/src/applications/transactions/controller/PhabricatorApplicationTransactionValueController.php index ef5e168898..d844465b45 100644 --- a/src/applications/transactions/controller/PhabricatorApplicationTransactionValueController.php +++ b/src/applications/transactions/controller/PhabricatorApplicationTransactionValueController.php @@ -37,7 +37,6 @@ final class PhabricatorApplicationTransactionValueController break; default: return new Aphront404Response(); - break; } if ($type == 'old') { diff --git a/src/applications/transactions/storage/PhabricatorApplicationTransaction.php b/src/applications/transactions/storage/PhabricatorApplicationTransaction.php index 5c70f59a89..ccf279e416 100644 --- a/src/applications/transactions/storage/PhabricatorApplicationTransaction.php +++ b/src/applications/transactions/storage/PhabricatorApplicationTransaction.php @@ -678,7 +678,6 @@ abstract class PhabricatorApplicationTransaction } else { return false; } - break; case PhabricatorTransactions::TYPE_CUSTOMFIELD: $field = $this->getTransactionCustomField(); if ($field) { @@ -705,7 +704,6 @@ abstract class PhabricatorApplicationTransaction return true; } return false; - break; default: break; } @@ -1061,7 +1059,6 @@ abstract class PhabricatorApplicationTransaction '%s updated subscribers...', $this->renderHandleLink($author_phid)); } - break; case PhabricatorTransactions::TYPE_FILE: $add = array_diff_key($new, $old); $add = array_keys($add); @@ -1179,7 +1176,6 @@ abstract class PhabricatorApplicationTransaction $this->renderHandleLink($author_phid)); } - break; case PhabricatorTransactions::TYPE_EDGE: $record = PhabricatorEdgeChangeRecord::newFromTransaction($this); $add = $record->getAddedPHIDs(); @@ -1275,7 +1271,6 @@ abstract class PhabricatorApplicationTransaction $this->renderHandleLink($author_phid), new PhutilNumber($undone)); } - break; case PhabricatorTransactions::TYPE_COLUMNS: $moves = $this->getInterestingMoves($new); @@ -1315,8 +1310,6 @@ abstract class PhabricatorApplicationTransaction phutil_count($moves), phutil_implode_html(', ', $fragments)); } - break; - case PhabricatorTransactions::TYPE_MFA: return pht( @@ -1490,7 +1483,6 @@ abstract class PhabricatorApplicationTransaction phutil_count($moves), phutil_implode_html(', ', $fragments)); } - break; case PhabricatorTransactions::TYPE_MFA: return null; diff --git a/src/view/phui/PHUIHeaderView.php b/src/view/phui/PHUIHeaderView.php index 8b8774c4cf..a2fdcaa81a 100644 --- a/src/view/phui/PHUIHeaderView.php +++ b/src/view/phui/PHUIHeaderView.php @@ -376,7 +376,6 @@ final class PHUIHeaderView extends AphrontTagView { break; default: throw new Exception(pht('Incorrect Property Passed')); - break; } }