Adding identifier checks for js name.
Change-Id: Ib8f2a4838286dcb3e15dc173cb95cf41d3fb1adf
diff --git a/dev/core/src/com/google/gwt/dev/jjs/ast/HasJsInfo.java b/dev/core/src/com/google/gwt/dev/jjs/ast/HasJsInfo.java
index ca238a4..a799f33 100644
--- a/dev/core/src/com/google/gwt/dev/jjs/ast/HasJsInfo.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/ast/HasJsInfo.java
@@ -18,16 +18,10 @@
/**
* Abstracts JsInterop information for the AST nodes.
*/
-public interface HasJsInfo {
+public interface HasJsInfo extends HasJsName {
void setJsMemberInfo(String namespace, String name, boolean exported);
- String getJsName();
-
- String getJsNamespace();
-
- String getQualifiedJsName();
-
boolean isJsNative();
boolean isJsOverlay();
diff --git a/dev/core/src/com/google/gwt/dev/jjs/ast/HasJsName.java b/dev/core/src/com/google/gwt/dev/jjs/ast/HasJsName.java
new file mode 100644
index 0000000..bab517b
--- /dev/null
+++ b/dev/core/src/com/google/gwt/dev/jjs/ast/HasJsName.java
@@ -0,0 +1,28 @@
+/*
+ * Copyright 2015 Google Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not
+ * use this file except in compliance with the License. You may obtain a copy of
+ * the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package com.google.gwt.dev.jjs.ast;
+
+/**
+ * Abstracts JsInterop name related information for the AST nodes.
+ */
+public interface HasJsName {
+
+ String getJsName();
+
+ String getJsNamespace();
+
+ String getQualifiedJsName();
+}
diff --git a/dev/core/src/com/google/gwt/dev/jjs/ast/JDeclaredType.java b/dev/core/src/com/google/gwt/dev/jjs/ast/JDeclaredType.java
index cd7b255..cb88496 100755
--- a/dev/core/src/com/google/gwt/dev/jjs/ast/JDeclaredType.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/ast/JDeclaredType.java
@@ -50,7 +50,8 @@
* initialization, and use the private variable <code>clinitTarget</code>to keep track which
* initializer in the superclass chain needs to be called.
*/
-public abstract class JDeclaredType extends JReferenceType implements CanHaveSuppressedWarnings {
+public abstract class JDeclaredType extends JReferenceType
+ implements CanHaveSuppressedWarnings, HasJsName {
private boolean isJsFunction;
private boolean isJsType;
@@ -612,9 +613,16 @@
return null;
}
+ public String getJsName() {
+ return Strings.isNullOrEmpty(jsName) ? getSimpleName() : jsName;
+ }
+
+ public String getJsNamespace() {
+ return jsNamespace;
+ }
+
public String getQualifiedJsName() {
- String simpleJsName = Strings.isNullOrEmpty(jsName) ? getSimpleName() : jsName;
- return jsNamespace.isEmpty() ? simpleJsName : jsNamespace + "." + simpleJsName;
+ return jsNamespace.isEmpty() ? getJsName() : jsNamespace + "." + getJsName();
}
public NestedClassDisposition getClassDisposition() {
diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/JsInteropRestrictionChecker.java b/dev/core/src/com/google/gwt/dev/jjs/impl/JsInteropRestrictionChecker.java
index 0fb9de3..428fa62 100644
--- a/dev/core/src/com/google/gwt/dev/jjs/impl/JsInteropRestrictionChecker.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/impl/JsInteropRestrictionChecker.java
@@ -21,6 +21,7 @@
import com.google.gwt.dev.jjs.HasSourceInfo;
import com.google.gwt.dev.jjs.ast.CanHaveSuppressedWarnings;
import com.google.gwt.dev.jjs.ast.Context;
+import com.google.gwt.dev.jjs.ast.HasJsName;
import com.google.gwt.dev.jjs.ast.JClassType;
import com.google.gwt.dev.jjs.ast.JConstructor;
import com.google.gwt.dev.jjs.ast.JDeclarationStatement;
@@ -43,6 +44,7 @@
import com.google.gwt.dev.jjs.ast.JStatement;
import com.google.gwt.dev.jjs.ast.JType;
import com.google.gwt.dev.jjs.ast.JVisitor;
+import com.google.gwt.dev.js.JsUtils;
import com.google.gwt.dev.util.log.AbstractTreeLogger;
import com.google.gwt.thirdparty.guava.common.base.Predicate;
import com.google.gwt.thirdparty.guava.common.collect.FluentIterable;
@@ -235,6 +237,8 @@
return;
}
+ checkMemberQualifiedJsName(field);
+
if (field.needsDynamicDispatch()) {
checkLocalName(localNames, field);
} else if (!field.isJsNative()) {
@@ -257,6 +261,8 @@
return;
}
+ checkMemberQualifiedJsName(method);
+
if (method.needsDynamicDispatch()) {
if (!isSyntheticBridgeMethod(method)) {
checkLocalName(localNames, method);
@@ -384,6 +390,53 @@
}
}
+ private void checkMemberQualifiedJsName(JMember member) {
+ if (member instanceof JConstructor) {
+ // Constructors always inherit their name and namespace from the enclosing type.
+ // The corresponding checks are done for the type separately.
+ return;
+ }
+
+ checkJsName(member);
+
+ if (member.getJsNamespace().equals(member.getEnclosingType().getQualifiedJsName())) {
+ // Namespace set by the enclosing type has already been checked.
+ return;
+ }
+
+ if (member.needsDynamicDispatch()) {
+ logError(member, "Instance member %s cannot declare a namespace.",
+ getMemberDescription(member));
+ return;
+ }
+
+ checkJsNamespace(member);
+ }
+
+ private <T extends HasJsName & HasSourceInfo> void checkJsName(T item) {
+ String jsName = item.getJsName();
+ if (jsName.equals(JsInteropUtil.INVALID_JSNAME)) {
+ // Errors are reported for this case when local name is checked.
+ return;
+ }
+
+ if (jsName.isEmpty()) {
+ logError(item, "%s cannot have an empty name.", getDescription(item));
+ return;
+ }
+ if (!JsUtils.isValidJsIdentifier(jsName)) {
+ logError(item, "%s has invalid name '%s'.", getDescription(item), jsName);
+ return;
+ }
+ }
+
+ private <T extends HasJsName & HasSourceInfo> void checkJsNamespace(T item) {
+ String jsNamespace = item.getJsNamespace();
+ if (!jsNamespace.isEmpty() && !JsUtils.isValidJsQualifiedName(jsNamespace)) {
+ logError(item, "%s has invalid namespace '%s'.", getDescription(item), jsNamespace);
+ }
+ }
+
private void checkStaticJsPropertyCalls() {
new JVisitor() {
@Override
@@ -520,7 +573,10 @@
if (!checkJsType(type)) {
return;
}
+ checkJsName(type);
+ checkJsNamespace(type);
}
+
if (type.isJsNative()) {
if (!checkNativeJsType(type)) {
return;
@@ -701,6 +757,13 @@
.equals(potentiallyOverriddenMethod.getJsniSignature(false, false));
}
+ private static String getDescription(HasSourceInfo hasSourceInfo) {
+ if (hasSourceInfo instanceof JDeclaredType) {
+ return getTypeDescription((JDeclaredType) hasSourceInfo);
+ } else {
+ return getMemberDescription((JMember) hasSourceInfo);
+ }
+ }
private static String getMemberDescription(JMember member) {
if (member instanceof JField) {
return String.format("'%s'", JjsUtils.getReadableDescription(member));
@@ -718,6 +781,10 @@
return String.format("'%s'", JjsUtils.getReadableDescription(method));
}
+ private static String getTypeDescription(JDeclaredType type) {
+ return String.format("'%s'", JjsUtils.getReadableDescription(type));
+ }
+
private boolean isUnusableByJsSuppressed(CanHaveSuppressedWarnings x) {
return x.getSuppressedWarnings() != null &&
x.getSuppressedWarnings().contains(JsInteropUtil.UNUSABLE_BY_JS);
diff --git a/dev/core/src/com/google/gwt/dev/js/JsToStringGenerationVisitor.java b/dev/core/src/com/google/gwt/dev/js/JsToStringGenerationVisitor.java
index e8371a2..1f18c16 100644
--- a/dev/core/src/com/google/gwt/dev/js/JsToStringGenerationVisitor.java
+++ b/dev/core/src/com/google/gwt/dev/js/JsToStringGenerationVisitor.java
@@ -80,7 +80,6 @@
import java.util.Iterator;
import java.util.List;
import java.util.Set;
-import java.util.regex.Pattern;
/**
* Produces text output from a JavaScript AST.
@@ -116,14 +115,6 @@
*/
private static final int JSBLOCK_LINES_TO_PRINT = 3;
- /**
- * A variable name is valid if it contains only letters, numbers, _, $ and
- * does not begin with a number. There are actually other valid variable
- * names, such as ones that contain escaped Unicode characters, but we
- * surround those names with quotes in property initializers to be safe.
- */
- private static final Pattern VALID_NAME_PATTERN = Pattern.compile("[a-zA-Z_$][\\w$]*");
-
protected boolean needSemi = true;
private List<NamedRange> classRanges = new ArrayList<NamedRange>();
private NamedRange currentClassRange;
@@ -723,8 +714,8 @@
// labels can be either string, integral, or decimal literals
if (labelExpr instanceof JsStringLiteral) {
String propName = ((JsStringLiteral) labelExpr).getValue();
- if (VALID_NAME_PATTERN.matcher(propName).matches()
- && !JsProtectedNames.isKeyword(propName)) {
+ if (JsUtils.isValidJsIdentifier(propName) && !JsProtectedNames.isKeyword(propName)) {
+ // Print unquoted if the property name is a valid identifier.
p.print(propName);
break printLabel;
}
diff --git a/dev/core/src/com/google/gwt/dev/js/JsUtils.java b/dev/core/src/com/google/gwt/dev/js/JsUtils.java
index 97d9186..729c74e 100644
--- a/dev/core/src/com/google/gwt/dev/js/JsUtils.java
+++ b/dev/core/src/com/google/gwt/dev/js/JsUtils.java
@@ -42,6 +42,8 @@
import java.util.List;
+import java.util.regex.Pattern;
+
/**
* Utils for JS AST.
*/
@@ -245,6 +247,24 @@
return null;
}
+ /**
+ * A JavaScript identifier contains only letters, numbers, _, $ and does not begin with a number.
+ * There are actually other valid identifiers, such as ones that contain escaped Unicode
+ * characters but we disallow those for the time being.
+ */
+ public static boolean isValidJsIdentifier(String name) {
+ return JAVASCRIPT_VALID_IDENTIFIER_PATTERN.matcher(name).matches();
+ }
+
+ public static boolean isValidJsQualifiedName(String name) {
+ return JAVASCRIPT_VALID_QUALIFIED_NAME_PATTERN.matcher(name).matches();
+ }
+
+ private static final String VALID_JS_NAME_REGEX = "[a-zA-Z_$][\\w_$]*";
+ private static final Pattern JAVASCRIPT_VALID_QUALIFIED_NAME_PATTERN =
+ Pattern.compile(VALID_JS_NAME_REGEX + "(\\." + VALID_JS_NAME_REGEX + ")*");
+ private static final Pattern JAVASCRIPT_VALID_IDENTIFIER_PATTERN =
+ Pattern.compile(VALID_JS_NAME_REGEX);
private static final String CALL_STRING = StringInterner.get().intern("call");
private JsUtils() {
diff --git a/dev/core/test/com/google/gwt/dev/jjs/impl/JsInteropRestrictionCheckerTest.java b/dev/core/test/com/google/gwt/dev/jjs/impl/JsInteropRestrictionCheckerTest.java
index c17ca62..b9699ae 100644
--- a/dev/core/test/com/google/gwt/dev/jjs/impl/JsInteropRestrictionCheckerTest.java
+++ b/dev/core/test/com/google/gwt/dev/jjs/impl/JsInteropRestrictionCheckerTest.java
@@ -883,6 +883,57 @@
assertBuggySucceeds();
}
+ public void testJsNameInvalidNamesFails() {
+ addSnippetImport("jsinterop.annotations.JsType");
+ addSnippetImport("jsinterop.annotations.JsMethod");
+ addSnippetImport("jsinterop.annotations.JsProperty");
+ addSnippetClassDecl(
+ "@JsType(name = \"a.b.c\") public static class Buggy {",
+ " @JsMethod(name = \"34s\") public void m() {}",
+ " @JsProperty(name = \"s^\") public int m;",
+ " @JsProperty(name = \"\") public int n;",
+ "}");
+
+ assertBuggyFails(
+ "Line 6: 'EntryPoint.Buggy' has invalid name 'a.b.c'.",
+ "Line 7: 'void EntryPoint.Buggy.m()' has invalid name '34s'.",
+ "Line 8: 'int EntryPoint.Buggy.m' has invalid name 's^'.",
+ "Line 9: 'int EntryPoint.Buggy.n' cannot have an empty name.");
+ }
+
+ public void testJsNameInvalidNamespacesFails() {
+ addSnippetImport("jsinterop.annotations.JsType");
+ addSnippetImport("jsinterop.annotations.JsMethod");
+ addSnippetImport("jsinterop.annotations.JsProperty");
+ addSnippetClassDecl(
+ "@JsType(namespace = \"a.b.\") public static class Buggy {",
+ " @JsMethod(namespace = \"34s\") public static void m() {}",
+ " @JsProperty(namespace = \"s^\") public static int n;",
+ " @JsProperty(namespace = \"\") public int p;",
+ " @JsMethod(namespace = \"a\") public void q() {}",
+ "}");
+
+ assertBuggyFails(
+ "Line 6: 'EntryPoint.Buggy' has invalid namespace 'a.b.'.",
+ "Line 7: 'void EntryPoint.Buggy.m()' has invalid namespace '34s'.",
+ "Line 8: 'int EntryPoint.Buggy.n' has invalid namespace 's^'.",
+ "Line 9: Instance member 'int EntryPoint.Buggy.p' cannot declare a namespace.",
+ "Line 10: Instance member 'void EntryPoint.Buggy.q()' cannot declare a namespace.");
+ }
+
+ public void testJsNameEmptyNamespacesSucceeds() throws Exception {
+ addSnippetImport("jsinterop.annotations.JsType");
+ addSnippetImport("jsinterop.annotations.JsMethod");
+ addSnippetImport("jsinterop.annotations.JsProperty");
+ addSnippetClassDecl(
+ "@JsType(namespace = \"\") public static class Buggy {",
+ " @JsMethod(namespace = \"\") public static void m() {}",
+ " @JsProperty(namespace = \"\") public static int n;",
+ "}");
+
+ assertBuggySucceeds();
+ }
+
public void testSingleJsTypeSucceeds() throws Exception {
addSnippetImport("jsinterop.annotations.JsType");
addSnippetClassDecl(
