Skip to content
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

Simplify usage of FileParserFactory #408

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

hgraca
Copy link
Contributor

@hgraca hgraca commented Sep 18, 2023

By allowing createFileParser to be called with null we need less code everywhere else to create a default file parser.

By allowing createFileParser to be called with null
we need less code everywhere else to create a default file parser.
public static function createFileParser(TargetPhpVersion $targetPhpVersion, bool $parseCustomAnnotations = true): FileParser
{
public static function createFileParser(
TargetPhpVersion $targetPhpVersion = null,
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
TargetPhpVersion $targetPhpVersion = null,
?TargetPhpVersion $targetPhpVersion = null,

I assume this should be explicitly marked as a nullable type instead of depending on implicit PHP behaviour.

@@ -26,7 +26,7 @@ private function __construct(?string $version)
$this->version = $version;
}

public static function create(?string $version): self
public static function create(string $version = null): self
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
public static function create(string $version = null): self
public static function create(?string $version = null): self

as above

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants