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

Regexp acceptor, fixes #200 #246

Merged
merged 9 commits into from May 14, 2019

Conversation

Projects
None yet
3 participants
@Protryon
Copy link
Member

commented Apr 17, 2019

Ported from shapesecurity/shift-parser-js#403

Supersedes #201.

@Protryon Protryon force-pushed the regex-acceptor branch from e1c6ab6 to d81d763 Apr 17, 2019

@Protryon Protryon requested review from Shilpi3 and michaelficarra Apr 18, 2019

@Protryon Protryon changed the title Regex acceptor, fixes #200 Regexp acceptor, fixes #200 Apr 19, 2019

}

public boolean verifyBackreferences() {
if (unicode) {

This comment has been minimized.

Copy link
@michaelficarra

michaelficarra Apr 23, 2019

Member

This is an instance var, so let's use this. so we know it's not static or local.

edit: Let's just do that consistently everywhere.

Protryon added some commits Apr 26, 2019

private static final String extendedSyntaxCharacters = "^$.*+?()[|";
private static final String[] controlCharacters = new String[]{"a", "b", "c", "d", "e", "f", "g", "h", "i", "j", "k", "l", "m", "n", "o", "p", "q", "r", "s", "t", "u", "v", "w", "x", "y", "z", "A", "B", "C", "D", "E", "F", "G", "H", "I", "J", "K", "L", "M", "N", "O", "P", "Q", "R", "S", "T", "U", "V", "W", "X", "Y", "Z"};

private static HashMap<String, Integer> constructControlEscapeCharacterValues() {

This comment has been minimized.

Copy link
@bakkot

bakkot Apr 30, 2019

Contributor

Do this with a static block:

private static final HashMap<String, Integer> controlEscapeCharacterValues = new HashMap<>();
static {
  controlEscapeCharacterValues.put("f", (int) '\f');
  // etc
}
}

@SafeVarargs
private final <B> F<Context, Maybe<B>> maybeLogicalOr(F<Context, Maybe<B>>... expressions) {

This comment has been minimized.

Copy link
@bakkot

bakkot Apr 30, 2019

Contributor

Let's call this anyOf, as in the JS implementation.

Protryon added some commits Apr 30, 2019

StringBuilder stringBuilder = new StringBuilder();

outer:
for (int i = 0; limit.isNothing() || limit.fromJust() > i; ++i) {

This comment has been minimized.

Copy link
@bakkot

bakkot May 1, 2019

Contributor

Let's pull out the isNothing and fromJust from the test, so it doesn't evaluate every time.

This comment has been minimized.

Copy link
@Protryon

Protryon May 1, 2019

Author Member

I see I am not the only one with no faith in the JVM. :D

Protryon added some commits May 1, 2019

"/|/",
"/}*/",
"/[\\99-\\98]/",
"/[\\99-\\100]/",

This comment has been minimized.

Copy link
@bakkot

bakkot May 3, 2019

Contributor

Let's get the tests in this commit over here as well.

@bakkot

bakkot approved these changes May 3, 2019

Copy link
Contributor

left a comment

LGTM other than missing tests.

@Protryon Protryon merged commit c8d8d10 into es2017 May 14, 2019

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.