-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Improve a few compile-time error messages #6385
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,4 +11,4 @@ try { | |
|
||
?> | ||
--EXPECT-- | ||
Illegal offset type | ||
Illegal array key type: Closure | ||
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,4 +11,4 @@ try { | |
|
||
?> | ||
--EXPECT-- | ||
Illegal offset type | ||
Illegal array key type: Closure |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,4 +7,4 @@ function foo () { | |
} | ||
?> | ||
--EXPECTF-- | ||
Fatal error: 'break' operator accepts only positive integers in %sbreak_error_001.php on line 3 | ||
Fatal error: The break statement accepts only an integer argument greater than or equal to 0 in %s on line %d | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The second half of the change is for consistency with the value error messages, but I don't insist on it too much in this case. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,4 +9,4 @@ use Blah\Exception; | |
use Blah\Ex; | ||
?> | ||
--EXPECTF-- | ||
Fatal error: Cannot use Blah\Ex as Ex because the name is already in use in %sbug42859.php on line 6 | ||
Fatal error: Cannot import class Blah\Ex as Ex, Ex has already been declared in %s on line %d | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So I normalized the different error messages related to the use statement to use |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,6 +12,6 @@ try { | |
var_dump($array); | ||
?> | ||
--EXPECT-- | ||
Illegal offset type | ||
Illegal array key type: array | ||
array(0) { | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,4 +7,4 @@ interface stdClass { } | |
|
||
?> | ||
--EXPECTF-- | ||
Fatal error: Cannot declare interface stdClass, because the name is already in use in %s on line %d | ||
Fatal error: interface stdClass has already been declared in %s on line %d | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wasn't sure if the |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -5,4 +5,4 @@ Error message for isset(func()) | |||||
isset(1 + 1); | ||||||
?> | ||||||
--EXPECTF-- | ||||||
Fatal error: Cannot use isset() on the result of an expression (you can use "null !== expression" instead) in %s on line %d | ||||||
Fatal error: Cannot use isset() on the result of an expression (you can use "expression !== null" instead) in %s on line %d | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I got rid of the Yoda-style condition, because is not the primary style, as far as I noticed so far (thankfully :D) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not super fan of the usage of
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks! This change makes sense for me! |
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a similar compile-time error message, so I decided to also change the run-time ones as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, leaving the
:
felt a little bit weird for me, but I'm ok to do so for consistency with other messages.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is that consistent with other messages though? I would expect this to be something like
Illegal array key of type Closure
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, not exactly. And the
Illegal array key of type Closure
format is the best; unfortunately, it didn't come to my mind :)Speaking about which branch to target: I was also unsure which branch to use. I first thought that the changes won't be very offensive, but it turned out that they are, so I agree, let's instead go for PHP 8.1.