add -warn-public-vars option to emit warning on use of public vars si…

…nce they do not work well when minified and used from MXML. There is an asdoc option @royalesuppresspublicvarwarning that can be used on a declaration or entire class to suppress the warning
apache · Feb 20, 2018 · eed5882 · eed5882
1 parent fb755ea
commit eed5882
Show file tree

Hide file tree

Showing 6 changed files with 125 additions and 1 deletion.
diff --git a/...ler-common/src/main/java/org/apache/royale/compiler/problems/PublicVarWarningProblem.java b/...ler-common/src/main/java/org/apache/royale/compiler/problems/PublicVarWarningProblem.java
@@ -0,0 +1,44 @@
+/*
+ *
+ *  Licensed to the Apache Software Foundation (ASF) under one or more
+ *  contributor license agreements.  See the NOTICE file distributed with
+ *  this work for additional information regarding copyright ownership.
+ *  The ASF licenses this file to You 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 org.apache.royale.compiler.problems;
+
+import org.apache.royale.compiler.common.ISourceLocation;
+import org.apache.royale.compiler.problems.annotations.DefaultSeverity;
+
+/**
+ *  Public vars don't work well in JS minified output.  The minifier
+ *  creates a "name" reference then gives the var a new minified name.
+ *  If the var is not read-only, the write to the var means the
+ *  reference is obsolete.  Also, MXML and States access attributes
+ *  by name which also breaks the reference.  Use getter/setters instead.
+ */
+@DefaultSeverity(CompilerProblemSeverity.WARNING)
+public class PublicVarWarningProblem extends CompilerProblem
+{
+    public static final String DESCRIPTION =
+        "public var may not work in minified JS output.  Use getter/setter instead.";
+
+    public static final int warningCode = 5044;
+
+    public PublicVarWarningProblem(ISourceLocation site)
+    {
+        super(site);
+    }
+}
diff --git a/compiler-jx/src/main/java/org/apache/royale/compiler/clients/MXMLJSC.java b/compiler-jx/src/main/java/org/apache/royale/compiler/clients/MXMLJSC.java
@@ -445,6 +445,7 @@ protected String[] removeJSArgs(String[] args)
     			  arg.startsWith("-compiler.js-define") ||
     			  arg.startsWith("-js-output") ||
     			  arg.startsWith("-js-load-config") ||
+    			  arg.startsWith("-warn-public-vars") ||
     			  arg.startsWith("-source-map")))
     			list.add(arg);						
     	}

diff --git a/...c/main/java/org/apache/royale/compiler/internal/codegen/js/royale/JSRoyaleDocEmitter.java b/...c/main/java/org/apache/royale/compiler/internal/codegen/js/royale/JSRoyaleDocEmitter.java
@@ -41,8 +41,11 @@
 import org.apache.royale.compiler.internal.codegen.js.jx.BindableEmitter;
 import org.apache.royale.compiler.internal.projects.RoyaleJSProject;
 import org.apache.royale.compiler.internal.scopes.ASScope;
+import org.apache.royale.compiler.problems.PublicVarWarningProblem;
 import org.apache.royale.compiler.projects.ICompilerProject;
+import org.apache.royale.compiler.tree.ASTNodeID;
 import org.apache.royale.compiler.tree.as.IASNode;
+import org.apache.royale.compiler.tree.as.IClassNode;
 import org.apache.royale.compiler.tree.as.IDefinitionNode;
 import org.apache.royale.compiler.tree.as.IExpressionNode;
 import org.apache.royale.compiler.tree.as.IFunctionNode;
@@ -422,6 +425,13 @@ else if (ns == IASKeywordConstants.PROTECTED)
         }
         else
         {
+        	RoyaleJSProject fjp =  (RoyaleJSProject)project;
+            boolean warnPublicVars = fjp.config != null && fjp.config.getWarnPublicVars();
+            if (warnPublicVars && !node.isConst() && !def.isBindable())
+            {
+                if (!suppressedWarning(node, fjp))
+                	fjp.getProblems().add(new PublicVarWarningProblem(node));
+            }
             emitPublic(node);
         }
 
@@ -443,5 +453,37 @@ public void emitPublic(IASNode node)
     	if (emitExports)
     		super.emitPublic(node);
     }
+
+    private boolean suppressedWarning(IVariableNode node, RoyaleJSProject fjp)
+    {
+    	boolean suppressed = false;
+        ASDocComment asDoc = (ASDocComment) node.getASDocComment();
+        boolean keepASDoc = fjp.config != null && fjp.config.getKeepASDoc();
+        String suppressToken = JSRoyaleEmitterTokens.SUPPRESS_PUBLIC_VAR_WARNING
+        .getToken();
+        if (asDoc != null && keepASDoc)
+        {
+            String docText = asDoc.commentNoEnd();
+            if (docText.contains(suppressToken))
+                return true;
+        }
+    	IASNode classNode = node.getParent().getParent();
+    	if (classNode == null)
+    		return false;
+    	if (classNode.getNodeID() == ASTNodeID.ClassID)
+    	{
+    		asDoc = (ASDocComment) ((IClassNode)classNode).getASDocComment();
+            if (asDoc != null && keepASDoc)
+            {
+                String docText = asDoc.commentNoEnd();
+                if (docText.contains(suppressToken))
+                    return true;
+            }
+            IClassDefinition cdef = ((IClassNode)classNode).getDefinition();
+            if (cdef.isBindable())
+            	return true;
+    	}
+    	return false;
+    }
 
 }
diff --git a/...ain/java/org/apache/royale/compiler/internal/codegen/js/royale/JSRoyaleEmitterTokens.java b/...ain/java/org/apache/royale/compiler/internal/codegen/js/royale/JSRoyaleEmitterTokens.java
@@ -45,6 +45,7 @@ public enum JSRoyaleEmitterTokens implements IEmitterTokens
     IGNORE_COERCION("@royaleignorecoercion"),
     IGNORE_IMPORT("@royaleignoreimport"),
     IGNORE_STRING_COERCION("@royalenoimplicitstringconversion"),
+    SUPPRESS_PUBLIC_VAR_WARNING("@royalesuppresspublicvarwarning"),
     PREINCREMENT("preincrement"),
     PREDECREMENT("predecrement"),
     POSTINCREMENT("postincrement"),

diff --git a/...ain/java/org/apache/royale/compiler/internal/driver/js/goog/JSGoogCompcConfiguration.java b/...ain/java/org/apache/royale/compiler/internal/driver/js/goog/JSGoogCompcConfiguration.java
@@ -422,6 +422,23 @@ public void setExportPublicSymbols(ConfigurationValue cv, boolean value)
     	exportPublicSymbols = value;
     }
 
-
+    //
+    // 'warn-public-vars'
+    //
+
+    private boolean warnPublicVars = true;
+
+    public boolean getWarnPublicVars()
+    {
+        return warnPublicVars;
+    }
+
+    @Config
+    @Mapping("warn-public-vars")
+    public void setWarnPublicVars(ConfigurationValue cv, boolean value)
+            throws ConfigurationException
+    {
+    	warnPublicVars = value;
+    }    
 
 }
diff --git a/...src/main/java/org/apache/royale/compiler/internal/driver/js/goog/JSGoogConfiguration.java b/...src/main/java/org/apache/royale/compiler/internal/driver/js/goog/JSGoogConfiguration.java
@@ -422,6 +422,25 @@ public void setExportPublicSymbols(ConfigurationValue cv, boolean value)
     }
 
 
+    //
+    // 'warn-public-vars'
+    //
+
+    private boolean warnPublicVars = true;
+
+    public boolean getWarnPublicVars()
+    {
+        return warnPublicVars;
+    }
+
+    @Config
+    @Mapping("warn-public-vars")
+    public void setWarnPublicVars(ConfigurationValue cv, boolean value)
+            throws ConfigurationException
+    {
+    	warnPublicVars = value;
+    }
+
 
 
 }