Fix System.getProperty to enable non-literal default values.

Change-Id: I5dc351304b2272077d42954a560af6220597ea0c Review-Link: https://gwt-review.googlesource.com/#/c/13160/
gwtproject · Jul 16, 2015 · 5e5d070 · 5e5d070
1 parent 829545d
commit 5e5d070
Show file tree

Hide file tree

Showing 4 changed files with 33 additions and 30 deletions.
diff --git a/dev/core/src/com/google/gwt/dev/jjs/ast/JPermutationDependentValue.java b/dev/core/src/com/google/gwt/dev/jjs/ast/JPermutationDependentValue.java
@@ -23,45 +23,46 @@
 /**
  * Represents a GWT.create() or a GWT.getProperty call before deferred binding decisions are
  * finalized. These nodes are replaced by the appropriate expressions by
- * {@link ResolvePermutationDependentValues}.
+ * {@link com.google.gwt.dev.jjs.impl.ResolvePermutationDependentValues}.
  */
 public class JPermutationDependentValue extends JExpression {
 
-  private enum Type { PROPERTY, TYPE_REBIND};
+  private enum Type { PROPERTY, TYPE_REBIND };
 
   private final List<JExpression> resultExpressions;
   private final List<String> resultValues;
-  private final String defaultValue;
+  private final JExpression defaultValueExpression;
   private final String requestedValue;
   private final Type valueType;
   private JType type;
 
   private JPermutationDependentValue(SourceInfo info, Type valueType, String requestedValue,
-      List<String> resultValues, String defaultValue, JType resultType,
+      List<String> resultValues, JExpression defaultValueExpression, JType resultType,
       List<JExpression> resultExpressions) {
     super(info);
     this.requestedValue = requestedValue;
     this.resultValues = resultValues;
     this.type = resultType;
     this.resultExpressions = resultExpressions;
     this.valueType = valueType;
-    this.defaultValue = defaultValue;
+    this.defaultValueExpression = defaultValueExpression;
   }
 
-  public static JPermutationDependentValue createTypeRebind(JProgram program,  SourceInfo info,
+  public static JPermutationDependentValue createTypeRebind(JProgram program, SourceInfo info,
       String requestedType, List<String> resultTypes, List<JExpression> instantiationExpressions) {
     return new JPermutationDependentValue(info, Type.TYPE_REBIND, requestedType, resultTypes, null,
         program.getTypeJavaLangObject(), instantiationExpressions);
   }
 
-  public static JPermutationDependentValue createRuntimeProperty(JProgram program,  SourceInfo info,
+  public static JPermutationDependentValue createRuntimeProperty(
-      String propertyName, String defaultValue) {
+      JProgram program, SourceInfo info, String propertyName, JExpression defaultValueExpression) {
-    return new JPermutationDependentValue(info, Type.PROPERTY, propertyName, null, defaultValue,
+    return new JPermutationDependentValue(info, Type.PROPERTY, propertyName, null,
-        program.getTypeJavaLangString(), Collections.<JExpression>emptyList());
+        defaultValueExpression, program.getTypeJavaLangString(),
+        Collections.<JExpression>emptyList());
   }
 
-  public String getDefaultValue() {
+  public JExpression getDefaultValueExpression() {
-    return defaultValue;
+    return defaultValueExpression;
   }
 
   public List<JExpression> getResultExpressions() {

diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/ResolvePermutationDependentValues.java b/dev/core/src/com/google/gwt/dev/jjs/impl/ResolvePermutationDependentValues.java
@@ -87,8 +87,13 @@ private JExpression propertyValueExpression(JPermutationDependentValue x) {
         return new JMethodCall(x.getSourceInfo(), null, method);
       }
 
-      return program.getLiteral(x.getSourceInfo(),
+      propertyValue = commonPropertyAndBindingInfo.getPropertyValue(x.getRequestedValue());
-          computePropertyValue(x.getRequestedValue(), x.getDefaultValue()));
+
+      if (propertyValue != null) {
+        return program.getLiteral(x.getSourceInfo(), propertyValue);
+      }
+
+      return x.getDefaultValueExpression();
     }
 
     private JExpression rebindClassExpression(JPermutationDependentValue x) {
@@ -151,11 +156,6 @@ public JExpression computeInstantiationExpression(JPermutationDependentValue x)
     return x.getResultExpressions().get(index);
   }
 
-  public String computePropertyValue(String propertyName, String defaultValue) {
-    String propertyValue = commonPropertyAndBindingInfo.getPropertyValue(propertyName);
-    return propertyValue == null ? defaultValue : propertyValue;
-  }
-
   private boolean execImpl() {
     ValueReplacer valueReplacer = new ValueReplacer();
     valueReplacer.accept(program);

diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/UnifyAst.java b/dev/core/src/com/google/gwt/dev/jjs/impl/UnifyAst.java
@@ -418,10 +418,9 @@ private JExpression handleSystemGetProperty(JMethodCall gwtGetPropertyCall) {
       JExpression defaultValueExpression = defaultVersionCalled ?
           gwtGetPropertyCall.getArgs().get(1) : null;
 
-      if (!(propertyNameExpression instanceof JStringLiteral) ||
+      if (!(propertyNameExpression instanceof JStringLiteral)) {
-          (defaultVersionCalled && !(defaultValueExpression instanceof JStringLiteral))) {
         error(gwtGetPropertyCall,
-            "Only string constants may be used as arguments to System.getProperty()");
+            "Only string constants may be used as property name in System.getProperty()");
         return null;
       }
       String propertyName = ((JStringLiteral) propertyNameExpression).getValue();
@@ -438,11 +437,12 @@ private JExpression handleSystemGetProperty(JMethodCall gwtGetPropertyCall) {
         return null;
       }
 
-      String defaultValue = defaultValueExpression == null ? null :
+      if (defaultValueExpression != null) {
-          ((JStringLiteral) defaultValueExpression).getValue();
+        defaultValueExpression = accept(defaultValueExpression);
-      return JPermutationDependentValue
+      }
-          .createRuntimeProperty(program, gwtGetPropertyCall.getSourceInfo(),
+
-              propertyName, defaultValue);
+      return JPermutationDependentValue.createRuntimeProperty(
+          program, gwtGetPropertyCall.getSourceInfo(), propertyName, defaultValueExpression);
     }
 
     private JExpression createRebindExpression(JMethodCall gwtCreateCall) {

diff --git a/user/test/com/google/gwt/dev/jjs/test/SystemGetPropertyTest.java b/user/test/com/google/gwt/dev/jjs/test/SystemGetPropertyTest.java
@@ -42,9 +42,11 @@ public void testBindingProperties() {
   public void testConfigurationProperties() {
     assertEquals("conf", System.getProperty("someConfigurationProperty"));
     assertEquals("conf", System.getProperty("someConfigurationProperty", "default"));
-  }
 
-  public void testDefaultValues() {
+    String someConf = System.getProperty("nonExistent", "default");
-    assertEquals("default", System.getProperty("nonExistent", "default"));
+    assertEquals("default", someConf);
+
+    // Note that default is not a String literal.
+    assertEquals("default", System.getProperty("otherNonExistent", someConf));
   }
 }