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

Improve exception message of ExtractionException #34

Closed
wants to merge 1 commit into from

Conversation

pierallard
Copy link
Contributor

When there was an issue on ClassNameExtractor or NamespaceExtractor, the Exception does not link to any file.
This PR improve the exception message to include the errored file.

@pierallard pierallard force-pushed the improve-error-message branch from a1757a0 to ce26755 Compare June 20, 2017 15:45
@@ -28,8 +28,8 @@ public function parse(\SplFileInfo $file)

$content = file_get_contents($file->getRealPath());
$tokens = Tokens::fromCode($content);
$classNamespace = $namespaceExtractor->extract($tokens);
$className = $classNameExtractor->extract($tokens);
$classNamespace = $namespaceExtractor->extract($tokens, $file);
Copy link
Contributor

Choose a reason for hiding this comment

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

i would catch the ExtractionException here and throw a new ParserException indicating the failing file path.

@pierallard pierallard force-pushed the improve-error-message branch 2 times, most recently from 3eabc88 to 225459c Compare June 20, 2017 16:06
/**
* @param \SplFileInfo $file
*
* @throws ParseException
Copy link
Contributor

Choose a reason for hiding this comment

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

ParseException or ParsingException ?

/**
* Class ParseException.
*
* @author Pierre Allard <pierre.alalrd@akeneo.com>
Copy link
Contributor

Choose a reason for hiding this comment

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

allard

@pierallard pierallard force-pushed the improve-error-message branch 4 times, most recently from af45442 to 392d04f Compare June 20, 2017 16:15
@pierallard pierallard force-pushed the improve-error-message branch from 392d04f to ffd1908 Compare June 20, 2017 16:16
Copy link
Contributor

@jjanvier jjanvier left a comment

Choose a reason for hiding this comment

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

I don't understand why you need to create a new exception for that, but ok

@pierallard
Copy link
Contributor Author

@jjanvier Because @nidup ask me to do this in his review :)

@nidup
Copy link
Contributor

nidup commented Jun 20, 2017

To avoid to pass the file in the extractor to throw a more accurate exception :)

$className = $classNameExtractor->extract($tokens);
try {
$classNamespace = $namespaceExtractor->extract($tokens, $file);
$className = $classNameExtractor->extract($tokens, $file);
Copy link
Contributor

@nidup nidup Jun 23, 2017

Choose a reason for hiding this comment

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

you pass a useless $file param here! (on both lines)

@jmleroux
Copy link
Contributor

won't do

@jmleroux jmleroux closed this Jan 23, 2025
@jmleroux jmleroux deleted the improve-error-message branch January 23, 2025 09:44
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.

4 participants