Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

fix($parse): block assigning to fields of a constructor prototype #14951

Closed
wants to merge 3 commits into from
Closed

fix($parse): block assigning to fields of a constructor prototype #14951

wants to merge 3 commits into from

Conversation

gkalpak
Copy link
Member

@gkalpak gkalpak commented Jul 26, 2016 •
edited
Loading

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

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

Does this PR introduce a breaking change?
No.

Please check if the PR fulfills these requirements

Other information:

  1. 1st commit: Add a few more tests for constructor access.
  2. 2nd commit: Fix + tests.
  3. 3rd commit: Add missing error page.

@gkalpak gkalpak force-pushed the fix-parse-block-assigning-to-constructor-prototypes branch from 5cf903b to aaf6226 Compare July 26, 2016 07:54
@gkalpak gkalpak added this to the 1.5.9 milestone Jul 26, 2016
@lgalfaso
Copy link
Contributor

I find the tests a little bit too long (maybe it could be split into cases using they). One way or another, this is good to land. LGTM

@gkalpak
Copy link
Member Author

gkalpak commented Jul 26, 2016

Thx, @lgalfaso! I'll break the tests up and merge.

@gkalpak gkalpak force-pushed the fix-parse-block-assigning-to-constructor-prototypes branch from aaf6226 to 81c6f55 Compare July 26, 2016 19:38
@gkalpak
Copy link
Member Author

gkalpak commented Jul 26, 2016

I split the tests up (and reverted the isecac-->isecac rename). I will merge once Travis is green.
Thx for the reviews, @lgalfaso and @petebacondarwin!

dcherman
dcherman reviewed Jul 26, 2016
View reviewed changes
src/ng/parse.js
throw $parseMinErr('isecaf',
'Assigning to a constructor is disallowed! Expression: {0}', fullExpression);
'Assigning to a constructor or it\'s prototype is disallowed! Expression: {0}',
Copy link
Contributor

Choose a reason for hiding this comment

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

its rather than it's :)

http://grammarist.com/spelling/its-its/

Copy link
Member Author

Choose a reason for hiding this comment

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

Why is it that I can always find these typos in other people's PRs, but not mine 😕

@gkalpak gkalpak force-pushed the fix-parse-block-assigning-to-constructor-prototypes branch from 81c6f55 to a1e4d66 Compare July 26, 2016 19:57
@gkalpak gkalpak force-pushed the fix-parse-block-assigning-to-constructor-prototypes branch from a1e4d66 to ee3e197 Compare July 26, 2016 19:59
@gkalpak gkalpak closed this in 4fa214c Jul 27, 2016
gkalpak added a commit that referenced this pull request Jul 27, 2016
@gkalpak
fix($parse): block assigning to fields of a constructor prototype
d7e31b5 
This commit also adds the missing `isecaf` error page and more tests for assignment to constructors.

Fixes #14939

Closes #14951
@gkalpak gkalpak deleted the fix-parse-block-assigning-to-constructor-prototypes branch July 27, 2016 08:04
@gkalpak gkalpak mentioned this pull request Jul 27, 2016
Closed
3 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees

@lgalfaso lgalfaso

Labels
cla: yes component: $parse frequency: low needs: review severity: inconvenient type: bug
Projects
None yet
Milestone
1.5.9
Development

Successfully merging this pull request may close these issues.
4 participants
@gkalpak @lgalfaso @dcherman @googlebot