This is an automated email from the ASF dual-hosted git repository.

gregdove pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/royale-compiler.git


The following commit(s) were added to refs/heads/develop by this push:
     new 9b7467b  Fix for file-private definitions output. This was an issue 
when using imports in the main package scope if the externally visible 
definition was a function, variable or namespace. Added a warning if using 
file-private definitions when the main definition is a variable. This seems 
highly unusual as a real use-case, and is probably not going to work in the 
same way as it can for function and namespace (based on local testing and 
observation)
9b7467b is described below

commit 9b7467b4506ff6cd7ee6a65278ac3f427adaaa17
Author: greg-dove <[email protected]>
AuthorDate: Sat Jun 22 16:12:20 2019 +1200

    Fix for file-private definitions output. This was an issue when using 
imports in the main package scope if the externally visible definition was a 
function, variable or namespace.
    Added a warning if using file-private definitions when the main definition 
is a variable. This seems highly unusual as a real use-case, and is probably 
not going to work in the same way as it can for function and namespace (based 
on local testing and observation)
---
 .../FilePrivateItemsWithMainVarWarningProblem.java |  50 +++++++++
 .../codegen/js/royale/JSRoyaleEmitter.java         | 119 ++++++++++++---------
 .../codegen/js/royale/TestRoyalePackage.java       |   2 +
 3 files changed, 118 insertions(+), 53 deletions(-)

diff --git 
a/compiler-common/src/main/java/org/apache/royale/compiler/problems/FilePrivateItemsWithMainVarWarningProblem.java
 
b/compiler-common/src/main/java/org/apache/royale/compiler/problems/FilePrivateItemsWithMainVarWarningProblem.java
new file mode 100644
index 0000000..10e8db7
--- /dev/null
+++ 
b/compiler-common/src/main/java/org/apache/royale/compiler/problems/FilePrivateItemsWithMainVarWarningProblem.java
@@ -0,0 +1,50 @@
+/*
+ *
+ *  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;
+
+/**
+ *  When a variable is the externally accessible package scoped definition, 
there is a
+ *  namespace implementation issue in javascript for other file-private 
members.
+ *  This use case seems limited, as it is likely that the externally visible 
variable
+ *  definition would need to depend on the file-private definitions, otherwise 
it is
+ *  questionable why they would even exist. Given the likelihood that this is 
very limited,
+ *  it is currently not recommended.
+ */
+@DefaultSeverity(CompilerProblemSeverity.WARNING)
+public class FilePrivateItemsWithMainVarWarningProblem extends CompilerProblem
+{
+    public static final String DESCRIPTION =
+        "Using file-private definitions outside a package level variable is 
not recommended and may not work (javascript).";
+    
+    public static final int warningCode = 5045;
+    
+    public FilePrivateItemsWithMainVarWarningProblem(ISourceLocation site)
+    {
+        super(site);
+    }
+    
+    public FilePrivateItemsWithMainVarWarningProblem(String sourcePath, int 
start, int end, int line, int column, int endLine, int endColumn)
+    {
+        super(sourcePath, start, end, line, column, endLine, endColumn);
+    }
+    
+}
diff --git 
a/compiler-jx/src/main/java/org/apache/royale/compiler/internal/codegen/js/royale/JSRoyaleEmitter.java
 
b/compiler-jx/src/main/java/org/apache/royale/compiler/internal/codegen/js/royale/JSRoyaleEmitter.java
index 5e82d8e..9523f29 100644
--- 
a/compiler-jx/src/main/java/org/apache/royale/compiler/internal/codegen/js/royale/JSRoyaleEmitter.java
+++ 
b/compiler-jx/src/main/java/org/apache/royale/compiler/internal/codegen/js/royale/JSRoyaleEmitter.java
@@ -72,7 +72,6 @@ import 
org.apache.royale.compiler.internal.codegen.js.jx.BinaryOperatorEmitter.D
 import org.apache.royale.compiler.internal.codegen.js.utils.EmitterUtils;
 import 
org.apache.royale.compiler.internal.codegen.mxml.royale.MXMLRoyaleEmitter;
 import org.apache.royale.compiler.internal.definitions.AccessorDefinition;
-import org.apache.royale.compiler.internal.definitions.AppliedVectorDefinition;
 import org.apache.royale.compiler.internal.definitions.FunctionDefinition;
 import org.apache.royale.compiler.internal.definitions.VariableDefinition;
 import org.apache.royale.compiler.internal.embedding.EmbedData;
@@ -83,6 +82,7 @@ import 
org.apache.royale.compiler.internal.projects.RoyaleProject;
 import org.apache.royale.compiler.internal.semantics.SemanticUtils;
 import org.apache.royale.compiler.internal.tree.as.*;
 import org.apache.royale.compiler.problems.EmbedUnableToReadSourceProblem;
+import 
org.apache.royale.compiler.problems.FilePrivateItemsWithMainVarWarningProblem;
 import org.apache.royale.compiler.projects.ICompilerProject;
 import org.apache.royale.compiler.tree.ASTNodeID;
 import org.apache.royale.compiler.tree.as.IASNode;
@@ -311,7 +311,7 @@ public class JSRoyaleEmitter extends JSGoogEmitter 
implements IJSRoyaleEmitter
                        
sb.append(JSGoogEmitterTokens.ROYALE_STATIC_DEPENDENCY_LIST.getToken());
                        boolean firstDependency = true;
                        for (String staticName : staticUsedNames)
-                       {                                       
+                       {
                                if (!firstDependency)
                                        sb.append(",");
                                firstDependency = false;
@@ -458,7 +458,7 @@ public class JSRoyaleEmitter extends JSGoogEmitter 
implements IJSRoyaleEmitter
             emitHoistedVariablesCodeBlock(node);
 
         setEmittingHoistedNodes(node, false);
-    }   
+    }
 
     protected void emitHoistedFunctionsCodeBlock(IFunctionNode node)
     {
@@ -627,7 +627,7 @@ public class JSRoyaleEmitter extends JSGoogEmitter 
implements IJSRoyaleEmitter
     {
                INamespaceDefinition nsDef = 
def.getNamespaceReference().resolveNamespaceReference(getWalker().getProject());
                String uri = nsDef.getURI();
-               if (!def.getNamespaceReference().isLanguageNamespace() && 
!uri.equals(INamespaceConstants.AS3URI) && 
+               if (!def.getNamespaceReference().isLanguageNamespace() && 
!uri.equals(INamespaceConstants.AS3URI) &&
                                                
!nsDef.getBaseName().equals(ASEmitterTokens.PRIVATE.getToken()))
                        return true;
                return false;
@@ -695,7 +695,7 @@ public class JSRoyaleEmitter extends JSGoogEmitter 
implements IJSRoyaleEmitter
        else if (!isDoc)
        {
                if (getModel().inStaticInitializer)
-                       if (!staticUsedNames.contains(name) && 
!NativeUtils.isJSNative(name) 
+                       if (!staticUsedNames.contains(name) && 
!NativeUtils.isJSNative(name)
                                        && !isExternal(name) && 
!getModel().getCurrentClass().getQualifiedName().equals(name)
                                        && (getModel().primaryDefinitionQName 
== null
                                                || 
!getModel().primaryDefinitionQName.equals(name)))
@@ -728,76 +728,89 @@ public class JSRoyaleEmitter extends JSGoogEmitter 
implements IJSRoyaleEmitter
     
//--------------------------------------------------------------------------
     // Package Level
     
//--------------------------------------------------------------------------
-
+    
     @Override
     public void emitPackageHeader(IPackageDefinition definition)
     {
-       IPackageNode packageNode = definition.getNode();
-       IFileNode fileNode = (IFileNode) 
packageNode.getAncestorOfType(IFileNode.class);
+        IPackageNode packageNode = definition.getNode();
+        IFileNode fileNode = (IFileNode) 
packageNode.getAncestorOfType(IFileNode.class);
         int nodeCount = fileNode.getChildCount();
         String mainClassName = null;
+        Boolean mainClassNameisVar = false;
+        IASNode firstInternalContent = null;
         for (int i = 0; i < nodeCount; i++)
         {
-               IASNode pnode = fileNode.getChild(i);
-
-               if (pnode instanceof IPackageNode)
-               {
-                       IScopedNode snode = 
((IPackageNode)pnode).getScopedNode();
-                   int snodeCount = snode.getChildCount();
-                   for (int j = 0; j < snodeCount; j++)
-                   {
-                       IASNode cnode = snode.getChild(j);
-                       if (cnode instanceof IClassNode)
-                       {
-                               mainClassName = 
((IClassNode)cnode).getQualifiedName();
-                               break;
-                       }
-                       else if (j == 0)
-                       {
-                           if (cnode instanceof IFunctionNode)
-                           {
-                               mainClassName = 
((IFunctionNode)cnode).getQualifiedName();
-                           }
-                           else if (cnode instanceof INamespaceNode)
-                           {
-                               mainClassName = 
((INamespaceNode)cnode).getQualifiedName();
-                           }
-                           else if (cnode instanceof IVariableNode)
-                           {
-                               mainClassName = 
((IVariableNode)cnode).getQualifiedName();
-                           }
-                               
-                       }
-                   }
-               }
-               else if (pnode instanceof IClassNode)
-               {
-                       String className = 
((IClassNode)pnode).getQualifiedName();
-                       getModel().getInternalClasses().put(className, 
mainClassName + "." + className);
-               }
-               else if (pnode instanceof IInterfaceNode)
-               {
-                       String className = 
((IInterfaceNode)pnode).getQualifiedName();
-                       getModel().getInternalClasses().put(className, 
mainClassName + "." + className);
-               }
+            IASNode pnode = fileNode.getChild(i);
+            
+            if (pnode instanceof IPackageNode)
+            {
+                IScopedNode snode = ((IPackageNode)pnode).getScopedNode();
+                int snodeCount = snode.getChildCount();
+                for (int j = 0; j < snodeCount; j++)
+                {
+                    //there can only be one externally visible definition of 
either class, namespace, variable or function
+                    //and package scope does not permit other class level 
access modifiers, otherwise a compiler error has already occurred.
+                    //So mainClassName is derived from the first instance of 
any of these.
+                    IASNode cnode = snode.getChild(j);
+                    if (cnode instanceof IClassNode)
+                    {
+                        mainClassName = ((IClassNode)cnode).getQualifiedName();
+                        break;
+                    }
+                    else if (cnode instanceof IFunctionNode)
+                    {
+                        mainClassName = 
((IFunctionNode)cnode).getQualifiedName();
+                        break;
+                    }
+                    else if (cnode instanceof INamespaceNode)
+                    {
+                        mainClassName = 
((INamespaceNode)cnode).getQualifiedName();
+                        break;
+                    }
+                    else if (cnode instanceof IVariableNode)
+                    {
+                        mainClassName = 
((IVariableNode)cnode).getQualifiedName();
+                        mainClassNameisVar = true;
+                        break;
+                    }
+                }
+            }
+            else if (pnode instanceof IClassNode)
+            {
+                String className = ((IClassNode)pnode).getQualifiedName();
+                getModel().getInternalClasses().put(className, mainClassName + 
"." + className);
+                if (firstInternalContent == null) firstInternalContent = pnode;
+            }
+            else if (pnode instanceof IInterfaceNode)
+            {
+                String className = ((IInterfaceNode)pnode).getQualifiedName();
+                getModel().getInternalClasses().put(className, mainClassName + 
"." + className);
+                if (firstInternalContent == null) firstInternalContent = pnode;
+            }
             else if (pnode instanceof IFunctionNode)
             {
                 String className = ((IFunctionNode)pnode).getQualifiedName();
                 getModel().getInternalClasses().put(className, mainClassName + 
"." + className);
+                if (firstInternalContent == null) firstInternalContent = pnode;
             }
             else if (pnode instanceof INamespaceNode)
             {
                 String className = ((INamespaceNode)pnode).getQualifiedName();
                 getModel().getInternalClasses().put(className, mainClassName + 
"." + className);
+                if (firstInternalContent == null) firstInternalContent = pnode;
             }
             else if (pnode instanceof IVariableNode)
             {
                 String className = ((IVariableNode)pnode).getQualifiedName();
                 getModel().getInternalClasses().put(className, mainClassName + 
"." + className);
+                if (firstInternalContent == null) firstInternalContent = pnode;
             }
         }
-
+        if (mainClassNameisVar && firstInternalContent != null) {
+            getProblems().add(new 
FilePrivateItemsWithMainVarWarningProblem(firstInternalContent));
+        }
         packageHeaderEmitter.emit(definition);
+        
     }
 
     @Override
@@ -1247,10 +1260,10 @@ public class JSRoyaleEmitter extends JSGoogEmitter 
implements IJSRoyaleEmitter
                        if 
(EmitterUtils.writeE4xFilterNode(getWalker().getProject(), getModel(), node))
                                write("node.");
                write("attribute(");
-                       getWalker().walk(parentNode.getRightOperandNode());     
                
+                       getWalker().walk(parentNode.getRightOperandNode());
                write(")");
                }
-                       
+               
                return;
         }
 
diff --git 
a/compiler-jx/src/test/java/org/apache/royale/compiler/internal/codegen/js/royale/TestRoyalePackage.java
 
b/compiler-jx/src/test/java/org/apache/royale/compiler/internal/codegen/js/royale/TestRoyalePackage.java
index 2d9166c..53756f1 100644
--- 
a/compiler-jx/src/test/java/org/apache/royale/compiler/internal/codegen/js/royale/TestRoyalePackage.java
+++ 
b/compiler-jx/src/test/java/org/apache/royale/compiler/internal/codegen/js/royale/TestRoyalePackage.java
@@ -1252,6 +1252,8 @@ public class TestRoyalePackage extends TestGoogPackage
        public void testPackageQualified_ClassAndInternalStaticConst()
        {
                IFileNode node = compileAS("package foo.bar {\n" +
+                               //adding an unneeded import node here exposed a 
failing test at one point:
+                               "import foo.bar.*\n" +
                                "public function A():Number {\n" +
                                "    return Internal.x;\n" +
                                "}}\n" +

Reply via email to