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

fix(compiler): Improved error reporting of the static reflector. #9011

Closed
wants to merge 1 commit into from

Conversation

chuckjaz
Copy link
Contributor

@chuckjaz chuckjaz commented Jun 3, 2016

Please check if the PR fulfills these requirements

What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Other... Please describe:

What is the current behavior? (You can also link to an open issue here)

Errors generated by the StaticReflector used during offline compilation does not produce context to diagnose the reason the error occurred. E.g. #8978.

What is the new behavior?

Improved the error message by including more information, including positional information and well as provide more context when the error message is generated.

Does this PR introduce a breaking change?

  • Yes
  • No

Other information:

StaticReflector provides more context on errors reported by the
collector.

The metadata collector now records the line and character of the node that
caused it to report the error.

Includes other minor fixes to error reporting and a wording change.

@@ -175,14 +177,12 @@ export class MetadataCollector {
// or
// var [<identifier>[, <identifier}+] = <expression>;
// are not supported.
let varValue = {
__symbolc: 'error',
Copy link
Contributor

Choose a reason for hiding this comment

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

nice typo fix :)

chuckjaz added a commit to chuckjaz/angular that referenced this pull request Jun 4, 2016
StaticReflector provides more context on errors reported by the
collector.

The metadata collector now records the line and character of the node that
caused it to report the error.

Includes other minor fixes to error reporting and a wording change.

Fixes angular#8978
Closes angular#9011
@@ -390,7 +390,11 @@ export class StaticReflector implements ReflectorReader {
return context;
}
case "error":
throw new Error(expression['message']);
let message = expression['message'];
if (expression['line']) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you add a test for showing line/column numbers?

@tbosch tbosch added the action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews label Jun 6, 2016
chuckjaz added a commit to chuckjaz/angular that referenced this pull request Jun 7, 2016
StaticReflector provides more context on errors reported by the
collector.

The metadata collector now records the line and character of the node that
caused it to report the error.

Includes other minor fixes to error reporting and a wording change.

Fixes angular#8978
Closes angular#9011
@chuckjaz chuckjaz removed the action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews label Jun 7, 2016
StaticReflector provides more context on errors reported by the
collector.

The metadata collector now records the line and character of the node that
caused it to report the error.

Includes other minor fixes to error reporting and a wording change.

Fixes angular#8978
Closes angular#9011
@chuckjaz chuckjaz added the action: merge The PR is ready for merge by the caretaker label Jun 7, 2016
@chuckjaz chuckjaz closed this in cf3548a Jun 7, 2016
KiaraGrouwstra pushed a commit to KiaraGrouwstra/angular that referenced this pull request Jun 10, 2016
StaticReflector provides more context on errors reported by the
collector.

The metadata collector now records the line and character of the node that
caused it to report the error.

Includes other minor fixes to error reporting and a wording change.

Fixes angular#8978
Closes angular#9011
KiaraGrouwstra pushed a commit to KiaraGrouwstra/angular that referenced this pull request Jun 21, 2016
StaticReflector provides more context on errors reported by the
collector.

The metadata collector now records the line and character of the node that
caused it to report the error.

Includes other minor fixes to error reporting and a wording change.

Fixes angular#8978
Closes angular#9011
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 8, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker cla: yes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants