Remove "because reasons", because reasons (#221)

Was funny back then but maybe it's not anymore. Also it doesn't really indicate that the message could be changed. Also, the message will always end with a `.`, unless it ends with `?` or `!` already. Sorry if it breaks things for you, should have done this much earlier. Close #220
spaze · Nov 28, 2023 · 4b952b1 · 4b952b1
2 parents 1500f90 + b8b11f4
commit 4b952b1
Show file tree

Hide file tree

Showing 45 changed files with 344 additions and 276 deletions.
diff --git a/src/Calls/ShellExecCalls.php b/src/Calls/ShellExecCalls.php
@@ -69,7 +69,7 @@ public function processNode(Node $node, Scope $scope): array
 			null,
 			null,
 			$this->disallowedCalls,
-			'Using the backtick operator (`...`) is forbidden because shell_exec() is forbidden, %2$s%3$s'
+			'Using the backtick operator (`...`) is forbidden because shell_exec() is forbidden%2$s%3$s'
 		);
 	}
 

diff --git a/src/DisallowedAttribute.php b/src/DisallowedAttribute.php
@@ -67,9 +67,9 @@ public function getExcludes(): array
 	}
 
 
-	public function getMessage(): string
+	public function getMessage(): ?string
 	{
-		return $this->message ?? 'because reasons';
+		return $this->message;
 	}
 
 

diff --git a/src/DisallowedCall.php b/src/DisallowedCall.php
@@ -82,9 +82,9 @@ public function getDefinedIn(): array
 	}
 
 
-	public function getMessage(): string
+	public function getMessage(): ?string
 	{
-		return $this->message ?? 'because reasons';
+		return $this->message;
 	}
 
 

diff --git a/src/DisallowedConstant.php b/src/DisallowedConstant.php
@@ -58,9 +58,9 @@ public function getConstant(): string
 	}
 
 
-	public function getMessage(): string
+	public function getMessage(): ?string
 	{
-		return $this->message ?? 'because reasons';
+		return $this->message;
 	}
 
 

diff --git a/src/DisallowedNamespace.php b/src/DisallowedNamespace.php
@@ -73,9 +73,9 @@ public function getExcludes(): array
 	}
 
 
-	public function getMessage(): string
+	public function getMessage(): ?string
 	{
-		return $this->message ?? 'because reasons';
+		return $this->message;
 	}
 
 

diff --git a/src/Formatter/Formatter.php b/src/Formatter/Formatter.php
@@ -41,4 +41,16 @@ public function formatIdentifier(array $identifiers): string
 		}
 	}
 
+
+	public function formatDisallowedMessage(?string $message): string
+	{
+		if (!$message) {
+			return '.';
+		}
+		if ($message[-1] !== '?' && $message[-1] !== '!') {
+			$message = rtrim($message, '.') . '.';
+		}
+		return ', ' . $message;
+	}
+
 }
diff --git a/src/RuleErrors/DisallowedAttributeRuleErrors.php b/src/RuleErrors/DisallowedAttributeRuleErrors.php
@@ -9,6 +9,7 @@
 use PHPStan\Rules\RuleErrorBuilder;
 use Spaze\PHPStan\Rules\Disallowed\Allowed\Allowed;
 use Spaze\PHPStan\Rules\Disallowed\DisallowedAttribute;
+use Spaze\PHPStan\Rules\Disallowed\Formatter\Formatter;
 use Spaze\PHPStan\Rules\Disallowed\Identifier\Identifier;
 
 class DisallowedAttributeRuleErrors
@@ -20,11 +21,15 @@ class DisallowedAttributeRuleErrors
 	/** @var Identifier */
 	private $identifier;
 
+	/** @var Formatter */
+	private $formatter;
 
-	public function __construct(Allowed $allowed, Identifier $identifier)
+
+	public function __construct(Allowed $allowed, Identifier $identifier, Formatter $formatter)
 	{
 		$this->allowed = $allowed;
 		$this->identifier = $identifier;
+		$this->formatter = $formatter;
 	}
 
 
@@ -46,9 +51,9 @@ public function get(Attribute $attribute, Scope $scope, array $disallowedAttribu
 			}
 
 			$errorBuilder = RuleErrorBuilder::message(sprintf(
-				'Attribute %s is forbidden, %s%s',
+				'Attribute %s is forbidden%s%s',
 				$attributeName,
-				$disallowedAttribute->getMessage(),
+				$this->formatter->formatDisallowedMessage($disallowedAttribute->getMessage()),
 				$disallowedAttribute->getAttribute() !== $attributeName ? " [{$attributeName} matches {$disallowedAttribute->getAttribute()}]" : ''
 			));
 			if ($disallowedAttribute->getErrorIdentifier()) {

diff --git a/src/RuleErrors/DisallowedCallsRuleErrors.php b/src/RuleErrors/DisallowedCallsRuleErrors.php
@@ -11,6 +11,7 @@
 use Spaze\PHPStan\Rules\Disallowed\Allowed\Allowed;
 use Spaze\PHPStan\Rules\Disallowed\DisallowedCall;
 use Spaze\PHPStan\Rules\Disallowed\File\FilePath;
+use Spaze\PHPStan\Rules\Disallowed\Formatter\Formatter;
 use Spaze\PHPStan\Rules\Disallowed\Identifier\Identifier;
 
 class DisallowedCallsRuleErrors
@@ -25,12 +26,16 @@ class DisallowedCallsRuleErrors
 	/** @var FilePath */
 	private $filePath;
 
+	/** @var Formatter */
+	private $formatter;
 
-	public function __construct(Allowed $allowed, Identifier $identifier, FilePath $filePath)
+
+	public function __construct(Allowed $allowed, Identifier $identifier, FilePath $filePath, Formatter $formatter)
 	{
 		$this->allowed = $allowed;
 		$this->identifier = $identifier;
 		$this->filePath = $filePath;
+		$this->formatter = $formatter;
 	}
 
 
@@ -54,9 +59,9 @@ public function get(?CallLike $node, Scope $scope, string $name, ?string $displa
 				&& !$this->allowed->isAllowed($scope, isset($node) ? $node->getArgs() : null, $disallowedCall)
 			) {
 				$errorBuilder = RuleErrorBuilder::message(sprintf(
-					$message ?? 'Calling %s is forbidden, %s%s',
+					$message ?? 'Calling %s is forbidden%s%s',
 					($displayName && $displayName !== $name) ? "{$name}() (as {$displayName}())" : "{$name}()",
-					$disallowedCall->getMessage(),
+					$this->formatter->formatDisallowedMessage($disallowedCall->getMessage()),
 					$disallowedCall->getCall() !== $name ? " [{$name}() matches {$disallowedCall->getCall()}()]" : ''
 				));
 				if ($disallowedCall->getErrorIdentifier()) {

diff --git a/src/RuleErrors/DisallowedConstantRuleErrors.php b/src/RuleErrors/DisallowedConstantRuleErrors.php
@@ -9,6 +9,7 @@
 use PHPStan\ShouldNotHappenException;
 use Spaze\PHPStan\Rules\Disallowed\Allowed\AllowedPath;
 use Spaze\PHPStan\Rules\Disallowed\DisallowedConstant;
+use Spaze\PHPStan\Rules\Disallowed\Formatter\Formatter;
 
 class DisallowedConstantRuleErrors
 {
@@ -17,10 +18,14 @@ class DisallowedConstantRuleErrors
 	/** @var AllowedPath */
 	private $allowedPath;
 
+	/** @var Formatter */
+	private $formatter;
 
-	public function __construct(AllowedPath $allowedPath)
+
+	public function __construct(AllowedPath $allowedPath, Formatter $formatter)
 	{
 		$this->allowedPath = $allowedPath;
+		$this->formatter = $formatter;
 	}
 
 
@@ -37,10 +42,10 @@ public function get(string $constant, Scope $scope, ?string $displayName, array
 		foreach ($disallowedConstants as $disallowedConstant) {
 			if ($disallowedConstant->getConstant() === $constant && !$this->allowedPath->isAllowedPath($scope, $disallowedConstant)) {
 				$errorBuilder = RuleErrorBuilder::message(sprintf(
-					'Using %s%s is forbidden, %s',
+					'Using %s%s is forbidden%s',
 					$disallowedConstant->getConstant(),
 					$displayName && $displayName !== $disallowedConstant->getConstant() ? ' (as ' . $displayName . ')' : '',
-					$disallowedConstant->getMessage()
+					$this->formatter->formatDisallowedMessage($disallowedConstant->getMessage())
 				));
 				if ($disallowedConstant->getErrorIdentifier()) {
 					$errorBuilder->identifier($disallowedConstant->getErrorIdentifier());

diff --git a/src/RuleErrors/DisallowedNamespaceRuleErrors.php b/src/RuleErrors/DisallowedNamespaceRuleErrors.php
@@ -8,6 +8,7 @@
 use PHPStan\Rules\RuleErrorBuilder;
 use Spaze\PHPStan\Rules\Disallowed\Allowed\AllowedPath;
 use Spaze\PHPStan\Rules\Disallowed\DisallowedNamespace;
+use Spaze\PHPStan\Rules\Disallowed\Formatter\Formatter;
 use Spaze\PHPStan\Rules\Disallowed\Identifier\Identifier;
 
 class DisallowedNamespaceRuleErrors
@@ -19,11 +20,15 @@ class DisallowedNamespaceRuleErrors
 	/** @var Identifier */
 	private $identifier;
 
+	/** @var Formatter */
+	private $formatter;
 
-	public function __construct(AllowedPath $allowedPath, Identifier $identifier)
+
+	public function __construct(AllowedPath $allowedPath, Identifier $identifier, Formatter $formatter)
 	{
 		$this->allowedPath = $allowedPath;
 		$this->identifier = $identifier;
+		$this->formatter = $formatter;
 	}
 
 
@@ -46,10 +51,10 @@ public function getDisallowedMessage(string $namespace, string $description, Sco
 			}
 
 			$errorBuilder = RuleErrorBuilder::message(sprintf(
-				'%s %s is forbidden, %s%s',
+				'%s %s is forbidden%s%s',
 				$description,
 				$namespace,
-				$disallowedNamespace->getMessage(),
+				$this->formatter->formatDisallowedMessage($disallowedNamespace->getMessage()),
 				$disallowedNamespace->getNamespace() !== $namespace ? " [{$namespace} matches {$disallowedNamespace->getNamespace()}]" : ''
 			));
 			if ($disallowedNamespace->getErrorIdentifier()) {

diff --git a/src/RuleErrors/DisallowedVariableRuleErrors.php b/src/RuleErrors/DisallowedVariableRuleErrors.php
@@ -9,17 +9,22 @@
 use PHPStan\ShouldNotHappenException;
 use Spaze\PHPStan\Rules\Disallowed\Allowed\AllowedPath;
 use Spaze\PHPStan\Rules\Disallowed\DisallowedVariable;
+use Spaze\PHPStan\Rules\Disallowed\Formatter\Formatter;
 
 class DisallowedVariableRuleErrors
 {
 
 	/** @var AllowedPath */
 	private $allowedPath;
 
+	/** @var Formatter */
+	private $formatter;
 
-	public function __construct(AllowedPath $allowedPath)
+
+	public function __construct(AllowedPath $allowedPath, Formatter $formatter)
 	{
 		$this->allowedPath = $allowedPath;
+		$this->formatter = $formatter;
 	}
 
 
@@ -35,9 +40,9 @@ public function get(string $variable, Scope $scope, array $disallowedVariables):
 		foreach ($disallowedVariables as $disallowedVariable) {
 			if ($disallowedVariable->getVariable() === $variable && !$this->allowedPath->isAllowedPath($scope, $disallowedVariable)) {
 				$errorBuilder = RuleErrorBuilder::message(sprintf(
-					'Using %s is forbidden, %s',
+					'Using %s is forbidden%s',
 					$disallowedVariable->getVariable(),
-					$disallowedVariable->getMessage()
+					$this->formatter->formatDisallowedMessage($disallowedVariable->getMessage())
 				));
 				if ($disallowedVariable->getErrorIdentifier()) {
 					$errorBuilder->identifier($disallowedVariable->getErrorIdentifier());

diff --git a/src/Usages/ClassConstantUsages.php b/src/Usages/ClassConstantUsages.php
@@ -105,7 +105,7 @@ function (ConstantStringType $constantString): string {
 			} elseif ($type->hasConstant($constant)->no()) {
 				return [
 					RuleErrorBuilder::message(sprintf(
-						'Cannot access constant %s on %s',
+						'Cannot access constant %s on %s.',
 						$constant,
 						$type->describe(VerbosityLevel::getRecommendedLevelByType($type))
 					))->build(),

diff --git a/tests/Calls/EchoCallsTest.php b/tests/Calls/EchoCallsTest.php
@@ -29,7 +29,7 @@ protected function getRule(): Rule
 		$filePath = new FilePath(new FileHelper(__DIR__));
 		$allowed = new Allowed($formatter, $normalizer, new AllowedPath($filePath));
 		return new EchoCalls(
-			new DisallowedCallsRuleErrors($allowed, new Identifier(), $filePath),
+			new DisallowedCallsRuleErrors($allowed, new Identifier(), $filePath, $formatter),
 			new DisallowedCallFactory($formatter, $normalizer, $allowed),
 			[
 				[
@@ -49,7 +49,7 @@ public function testRule(): void
 		// Based on the configuration above, in this file:
 		$this->analyse([__DIR__ . '/../src/disallowed/functionCalls.php'], [
 			[
-				'Calling echo() is forbidden, because reasons',
+				'Calling echo() is forbidden.',
 				42,
 			],
 		]);

diff --git a/tests/Calls/EmptyCallsTest.php b/tests/Calls/EmptyCallsTest.php
@@ -29,7 +29,7 @@ protected function getRule(): Rule
 		$filePath = new FilePath(new FileHelper(__DIR__));
 		$allowed = new Allowed($formatter, $normalizer, new AllowedPath($filePath));
 		return new EmptyCalls(
-			new DisallowedCallsRuleErrors($allowed, new Identifier(), $filePath),
+			new DisallowedCallsRuleErrors($allowed, new Identifier(), $filePath, $formatter),
 			new DisallowedCallFactory($formatter, $normalizer, $allowed),
 			[
 				[
@@ -49,7 +49,7 @@ public function testRule(): void
 		// Based on the configuration above, in this file:
 		$this->analyse([__DIR__ . '/../src/disallowed/functionCalls.php'], [
 			[
-				'Calling empty() is forbidden, because reasons',
+				'Calling empty() is forbidden.',
 				41,
 			],
 		]);

diff --git a/tests/Calls/EvalCallsTest.php b/tests/Calls/EvalCallsTest.php
@@ -29,7 +29,7 @@ protected function getRule(): Rule
 		$filePath = new FilePath(new FileHelper(__DIR__));
 		$allowed = new Allowed($formatter, $normalizer, new AllowedPath($filePath));
 		return new EvalCalls(
-			new DisallowedCallsRuleErrors($allowed, new Identifier(), $filePath),
+			new DisallowedCallsRuleErrors($allowed, new Identifier(), $filePath, $formatter),
 			new DisallowedCallFactory($formatter, $normalizer, $allowed),
 			[
 				[
@@ -49,7 +49,7 @@ public function testRule(): void
 		// Based on the configuration above, in this file:
 		$this->analyse([__DIR__ . '/../src/disallowed/functionCalls.php'], [
 			[
-				'Calling eval() is forbidden, because reasons',
+				'Calling eval() is forbidden.',
 				28,
 			],
 		]);

diff --git a/tests/Calls/ExitDieCallsTest.php b/tests/Calls/ExitDieCallsTest.php
@@ -29,7 +29,7 @@ protected function getRule(): Rule
 		$filePath = new FilePath(new FileHelper(__DIR__));
 		$allowed = new Allowed($formatter, $normalizer, new AllowedPath($filePath));
 		return new ExitDieCalls(
-			new DisallowedCallsRuleErrors($allowed, new Identifier(), $filePath),
+			new DisallowedCallsRuleErrors($allowed, new Identifier(), $filePath, $formatter),
 			new DisallowedCallFactory($formatter, $normalizer, $allowed),
 			[
 				[
@@ -52,19 +52,19 @@ public function testRule(): void
 		// Based on the configuration above, in this file:
 		$this->analyse([__DIR__ . '/../src/disallowed/functionCalls.php'], [
 			[
-				'Calling die() is forbidden, because reasons',
+				'Calling die() is forbidden.',
 				30,
 			],
 			[
-				'Calling die() is forbidden, because reasons',
+				'Calling die() is forbidden.',
 				33,
 			],
 			[
-				'Calling exit() is forbidden, because reasons',
+				'Calling exit() is forbidden.',
 				36,
 			],
 			[
-				'Calling exit() is forbidden, because reasons',
+				'Calling exit() is forbidden.',
 				39,
 			],
 		]);

diff --git a/tests/Calls/FunctionCallsAllowInFunctionsTest.php b/tests/Calls/FunctionCallsAllowInFunctionsTest.php
@@ -29,7 +29,7 @@ protected function getRule(): Rule
 		$filePath = new FilePath(new FileHelper(__DIR__));
 		$allowed = new Allowed($formatter, $normalizer, new AllowedPath($filePath));
 		return new FunctionCalls(
-			new DisallowedCallsRuleErrors($allowed, new Identifier(), $filePath),
+			new DisallowedCallsRuleErrors($allowed, new Identifier(), $filePath, $formatter),
 			new DisallowedCallFactory($formatter, $normalizer, $allowed),
 			$this->createReflectionProvider(),
 			[
@@ -56,7 +56,7 @@ public function testRule(): void
 		$this->analyse([__DIR__ . '/../libs/Functions.php'], [
 			[
 				// expect this error message:
-				'Calling sha1() is forbidden, because reasons [sha1() matches sha*()]',
+				'Calling sha1() is forbidden. [sha1() matches sha*()]',
 				// on this line:
 				15,
 			],

diff --git a/tests/Calls/FunctionCallsAllowInMethodsTest.php b/tests/Calls/FunctionCallsAllowInMethodsTest.php
@@ -29,7 +29,7 @@ protected function getRule(): Rule
 		$filePath = new FilePath(new FileHelper(__DIR__));
 		$allowed = new Allowed($formatter, $normalizer, new AllowedPath($filePath));
 		return new FunctionCalls(
-			new DisallowedCallsRuleErrors($allowed, new Identifier(), $filePath),
+			new DisallowedCallsRuleErrors($allowed, new Identifier(), $filePath, $formatter),
 			new DisallowedCallFactory($formatter, $normalizer, $allowed),
 			$this->createReflectionProvider(),
 			[
@@ -65,7 +65,7 @@ public function testRule(): void
 		$this->analyse([__DIR__ . '/../libs/Royale.php'], [
 			[
 				// expect this error message:
-				'Calling sha1() is forbidden, because reasons',
+				'Calling sha1() is forbidden.',
 				// on this line:
 				11,
 			],