Skip to content

Commit

Permalink
Support class-string
Browse files Browse the repository at this point in the history
  • Loading branch information
@ondrejmirtes
ondrejmirtes committed Sep 25, 2019
1 parent 3067163 commit 3e95614
Show file tree
Hide file tree
Showing 2 changed files with 61 additions and 1 deletion.
2 changes: 1 addition & 1 deletion src/Lexer/Lexer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ private function initialize(): void
self::TOKEN_SINGLE_QUOTED_STRING => '\'(?:\\\\[^\\r\\n]|[^\'\\r\\n\\\\])*+\'',
self::TOKEN_DOUBLE_QUOTED_STRING => '"(?:\\\\[^\\r\\n]|[^"\\r\\n\\\\])*+"',

self::TOKEN_IDENTIFIER => '(?:[\\\\]?+[a-z_\\x80-\\xFF][0-9a-z_\\x80-\\xFF]*+)++',
self::TOKEN_IDENTIFIER => '(?:[\\\\]?+[a-z_\\x80-\\xFF][0-9a-z_\\x80-\\xFF-]*+)++',
self::TOKEN_THIS_VARIABLE => '\\$this(?![0-9a-z_\\x80-\\xFF])',
self::TOKEN_VARIABLE => '\\$[a-z_\\x80-\\xFF][0-9a-z_\\x80-\\xFF]*+',

Expand Down
60 changes: 60 additions & 0 deletions tests/PHPStan/Parser/PhpDocParserTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2508,6 +2508,66 @@ public function provideExtendsTagsData(): \Iterator
),
]),
];

yield [
'class-string in @return',
'/** @return class-string */',
new PhpDocNode([
new PhpDocTagNode(
'@return',
new ReturnTagValueNode(
new IdentifierTypeNode('class-string'),
''
)
),
]),
];

yield [
'class-string in @return with description',
'/** @return class-string some description */',
new PhpDocNode([
new PhpDocTagNode(
'@return',
new ReturnTagValueNode(
new IdentifierTypeNode('class-string'),
'some description'
)
),
]),
];

yield [
'class-string in @param',
'/** @param class-string $test */',
new PhpDocNode([
new PhpDocTagNode(
'@param',
new ParamTagValueNode(
new IdentifierTypeNode('class-string'),
false,
'$test',
''
)
),
]),
];

yield [
'class-string in @param with description',
'/** @param class-string $test some description */',
new PhpDocNode([
new PhpDocTagNode(
'@param',
new ParamTagValueNode(
new IdentifierTypeNode('class-string'),
false,
'$test',
'some description'
)
),
]),
];
}

public function providerDebug(): \Iterator
Expand Down

7 comments on commit 3e95614

@ondrejmirtes
Copy link
Member Author

Choose a reason for hiding this comment

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

@arnaud-lb @JanTvrdik Please take a look. Is it fine for you?

@JanTvrdik
Copy link
Collaborator

Choose a reason for hiding this comment

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

@ondrejmirtes Nope. This will now allow - in all identifiers (such as class names).

@JanTvrdik
Copy link
Collaborator

Choose a reason for hiding this comment

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

class-string needs to have a custom token, similar to e.g. TOKEN_THIS_VARIABLE

@ondrejmirtes
Copy link
Member Author

Choose a reason for hiding this comment

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

I thought about it, but isn't it fine?

  1. There are classes like OCI-Lob.
  2. We detect unknown classes in PHPStan rules.

@JanTvrdik
Copy link
Collaborator

Choose a reason for hiding this comment

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

I thought about it, but isn't it fine?

It breaks responsibility chain (it pushes syntax error to semantic level).

There are classes like OCI-Lob.

Which are invalid and should never exist in the first place.

@ondrejmirtes
Copy link
Member Author

Choose a reason for hiding this comment

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

OK, I'll try to rework it.

@ondrejmirtes
Copy link
Member Author

Choose a reason for hiding this comment

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

I remembered one more reason why the current way is convenient: We can still use GenericTypeNode for class-string<T>. If class-string is reworked according to $this, I'd also have to create something like ThisTypeNode. Or it's not necessary?

Please sign in to comment.