Skip to content

Commit

Permalink
Introduce matchesName as an alternative to matchesQualifiedName. Curr…
Browse files Browse the repository at this point in the history
…ently, matchesQualifiedName is overused and we would like to start moving away from "QualifiedName" as a key concept. Additionally, matchesName this is more efficient than matchesQualifiedName for the case of matching NAME nodes. A follow up CL will replace the obvious places matchesQualifiedName can be replaced with matchesName.

While we are here add a few missing test cases for "super"

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=258170284
  • Loading branch information
concavelenz authored and tjgq committed Jul 17, 2019
1 parent 458c5ac commit 0db7072
Show file tree
Hide file tree
Showing 2 changed files with 78 additions and 2 deletions.
36 changes: 34 additions & 2 deletions src/com/google/javascript/rhino/Node.java
Expand Up @@ -2148,6 +2148,35 @@ public final boolean isQualifiedName() {
}
}

/**
* Returns whether a node matches a simple name, such as
* <code>x</code>, returns false if this is not a NAME node.
*/
public final boolean matchesName(String name) {
if (token != token.NAME) {
return false;
}
String internalString = getString();
return !internalString.isEmpty() && name.equals(internalString);
}

/**
* Check that if two NAME node match, returns false if either node is
* not a NAME node. As a empty string is not considered a
* valid Name (it is an AST placeholder), empty strings are never
* considered to be matches.
*/
@SuppressWarnings("ReferenceEquality")
public final boolean matchesName(Node n) {
if (token != token.NAME || n.token != Token.NAME) {
return false;
}

// ==, rather than equal as it is intern'd in setString
String internalString = getString();
return internalString != "" && internalString == n.getString();
}

/**
* Returns whether a node matches a simple or a qualified name, such as
* <code>x</code> or <code>a.b.c</code> or <code>this.a</code>.
Expand Down Expand Up @@ -2197,7 +2226,8 @@ public final boolean matchesQualifiedName(Node n) {
switch (token) {
case NAME:
// ==, rather than equal as it is intern'd in setString
return !getString().isEmpty() && getString() == n.getString();
String internalString = getString();
return internalString != "" && internalString == n.getString();
case THIS:
case SUPER:
return true;
Expand All @@ -2218,10 +2248,12 @@ public final boolean matchesQualifiedName(Node n) {
* a "this" reference, such as <code>a.b.c</code>, but not <code>this.a</code>
* .
*/
@SuppressWarnings("ReferenceEquality")
public final boolean isUnscopedQualifiedName() {
switch (this.getToken()) {
case NAME:
return !getString().isEmpty();
// Both string are intern'd.
return getString() != "";
case GETPROP:
return getFirstChild().isUnscopedQualifiedName();
default:
Expand Down
44 changes: 44 additions & 0 deletions test/com/google/javascript/rhino/NodeTest.java
Expand Up @@ -251,8 +251,19 @@ public void testMatchesQualifiedName1() {
assertThat(qname("this.b").matchesQualifiedName("a.b")).isFalse();
assertThat(qname("this.b").matchesQualifiedName(".b")).isFalse();
assertThat(qname("this.b").matchesQualifiedName("a.")).isFalse();
assertThat(qname("this.b").matchesQualifiedName("super.b")).isFalse();
assertThat(qname("this.b").matchesQualifiedName("this.b")).isTrue();

assertThat(qname("super").matchesQualifiedName("super")).isTrue();
assertThat(qname("super").matchesQualifiedName("superx")).isFalse();

assertThat(qname("super.b").matchesQualifiedName("a")).isFalse();
assertThat(qname("super.b").matchesQualifiedName("a.b")).isFalse();
assertThat(qname("super.b").matchesQualifiedName(".b")).isFalse();
assertThat(qname("super.b").matchesQualifiedName("a.")).isFalse();
assertThat(qname("super.b").matchesQualifiedName("this.b")).isFalse();
assertThat(qname("super.b").matchesQualifiedName("super.b")).isTrue();

assertThat(qname("a.b.c").matchesQualifiedName("a.b.c")).isTrue();
assertThat(qname("a.b.c").matchesQualifiedName("a.b.c")).isTrue();

Expand Down Expand Up @@ -293,8 +304,14 @@ public void testMatchesQualifiedName2() {

assertThat(qname("this.b").matchesQualifiedName(qname("a"))).isFalse();
assertThat(qname("this.b").matchesQualifiedName(qname("a.b"))).isFalse();
assertThat(qname("this.b").matchesQualifiedName(qname("super.b"))).isFalse();
assertThat(qname("this.b").matchesQualifiedName(qname("this.b"))).isTrue();

assertThat(qname("super.b").matchesQualifiedName(qname("a"))).isFalse();
assertThat(qname("super.b").matchesQualifiedName(qname("a.b"))).isFalse();
assertThat(qname("super.b").matchesQualifiedName(qname("this.b"))).isFalse();
assertThat(qname("super.b").matchesQualifiedName(qname("super.b"))).isTrue();

assertThat(qname("a.b.c").matchesQualifiedName(qname("a.b.c"))).isTrue();
assertThat(qname("a.b.c").matchesQualifiedName(qname("a.b.c"))).isTrue();

Expand Down Expand Up @@ -324,6 +341,28 @@ public void testMatchesQualifiedName2() {
assertThat(new Node(Token.INC, IR.name("x")).matchesQualifiedName(qname("x"))).isFalse();
}

@Test
public void testMatchesName() {
// Empty string are treat as unique.
assertThat(IR.name("").matchesName("")).isFalse();

assertThat(IR.name("a").matchesName("a")).isTrue();
assertThat(IR.name("a").matchesName("a.b")).isFalse();
assertThat(IR.name("a").matchesName("")).isFalse();

assertThat(IR.thisNode().matchesName("this")).isFalse();
assertThat(IR.superNode().matchesName("super")).isFalse();
}

@Test
public void testMatchesNameNodes() {
assertThat(IR.name("a").matchesName(qname("a"))).isTrue();
assertThat(IR.name("a").matchesName(qname("a.b"))).isFalse();

assertThat(IR.thisNode().matchesName(qname("this"))).isFalse();
assertThat(IR.superNode().matchesName(qname("super"))).isFalse();
}

public static Node qname(String name) {
int endPos = name.indexOf('.');
if (endPos == -1) {
Expand All @@ -333,6 +372,8 @@ public static Node qname(String name) {
String nodeName = name.substring(0, endPos);
if ("this".equals(nodeName)) {
node = IR.thisNode();
} else if ("super".equals(nodeName)) {
node = IR.superNode();
} else {
node = IR.name(nodeName);
}
Expand Down Expand Up @@ -504,8 +545,11 @@ public void testInvalidSourceOffset() {
public void testQualifiedName() {
assertThat(IR.name("").getQualifiedName()).isNull();
assertThat(IR.name("a").getQualifiedName()).isEqualTo("a");
assertThat(IR.thisNode().getQualifiedName()).isEqualTo("this");
assertThat(IR.superNode().getQualifiedName()).isEqualTo("super");
assertThat(IR.getprop(IR.name("a"), IR.string("b")).getQualifiedName()).isEqualTo("a.b");
assertThat(IR.getprop(IR.thisNode(), IR.string("b")).getQualifiedName()).isEqualTo("this.b");
assertThat(IR.getprop(IR.superNode(), IR.string("b")).getQualifiedName()).isEqualTo("super.b");
assertThat(IR.getprop(IR.call(IR.name("a")), IR.string("b")).getQualifiedName()).isNull();
}

Expand Down

0 comments on commit 0db7072

Please sign in to comment.