Skip to content

[Yaml][Lint] Add line numbers to JSON output. #23294

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Jun 29, 2017
Merged

[Yaml][Lint] Add line numbers to JSON output. #23294

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/Symfony/Bridge/Twig/Command/LintCommand.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ private function validate(Environment $twig, $template, $file)
} catch (Error $e) {
$twig->setLoader($realLoader);

return array('template' => $template, 'file' => $file, 'valid' => false, 'exception' => $e);
return array('template' => $template, 'file' => $file, 'line' => $e->getTemplateLine(), 'valid' => false, 'exception' => $e);
}

return array('template' => $template, 'file' => $file, 'valid' => true);
Expand Down
4 changes: 2 additions & 2 deletions src/Symfony/Component/Yaml/Command/LintCommand.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ private function validate($content, $file = null)
{
$prevErrorHandler = set_error_handler(function ($level, $message, $file, $line) use (&$prevErrorHandler) {
if (E_USER_DEPRECATED === $level) {
throw new ParseException($message);
throw new ParseException($message, $this->getParser()->getLastLineNumberBeforeDeprecation());
}

return $prevErrorHandler ? $prevErrorHandler($level, $message, $file, $line) : false;
Expand All @@ -114,7 +114,7 @@ private function validate($content, $file = null)
try {
$this->getParser()->parse($content, Yaml::PARSE_CONSTANT);
} catch (ParseException $e) {
return array('file' => $file, 'valid' => false, 'message' => $e->getMessage());
return array('file' => $file, 'line' => $e->getParsedLine(), 'valid' => false, 'message' => $e->getMessage());
Copy link
Member

Choose a reason for hiding this comment

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

getParsedLine() can return -1. Should we add special handling for that case?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes I've noticed that while working with this. I want to research it a little more because the message will have a line number sometimes, so I will try to find out why it is not returning for getParsedLine().

Copy link
Member

Choose a reason for hiding this comment

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

IMO, displaying the line number here all the time is fine. For cases having -1, the right handling would be to give them the right line number if we can

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've added getLastLineNumberBeforeDeprecation to get the last line read before the deprecation was triggered. Do you think this is the way to go? The only other way I see to get line numbers from these trigger_error is by adding them into the $error_msg and parse them out.. But this seems clearer even though not perfect (it's a very specific method). Maybe you think we should make getRealCurrentLineNb public?

} finally {
restore_error_handler();
}
Expand Down
10 changes: 10 additions & 0 deletions src/Symfony/Component/Yaml/Parser.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,16 @@ public function parse($value, $flags = 0)
return $data;
}

/**
* @internal
*
* @return int
*/
public function getLastLineNumberBeforeDeprecation()
{
return $this->getRealCurrentLineNb();
}

private function doParse($value, $flags)
{
$this->currentLineNb = -1;
Expand Down