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" +