This is an automated email from the ASF dual-hosted git repository. joshtynjala pushed a commit to branch develop in repository https://gitbox.apache.org/repos/asf/royale-compiler.git
commit d23dd1656907ef35d4cd1ee0a1d7be93f48d6a70 Author: Josh Tynjala <[email protected]> AuthorDate: Tue Apr 23 14:57:12 2024 -0700 CSSDocument: if CSSFontFace or CSSNamespace have syntax issues, report them as problems instead of throwing exceptions The exceptions were treated as unexpected, and they didn't even include a descriptive message of the issue (just something like NullPointerException), so it just looked like a compiler bug instead of trying to help users discover what was wrong with their CSS syntax --- .../apache/royale/compiler/internal/css/CSSTree.g | 25 +++++++++-- .../royale/compiler/internal/css/CSSFontFace.java | 42 ++++++++++++++---- .../internal/css/CSSNamespaceDefinition.java | 32 ++++++++++++-- .../problems/CSSNamespaceEmptyURIProblem.java | 37 ++++++++++++++++ .../problems/CSSRequiredDescriptorProblem.java | 42 ++++++++++++++++++ .../internal/css/CSSNamespaceDefinitionTests.java | 51 ++++++++++++++-------- 6 files changed, 195 insertions(+), 34 deletions(-) diff --git a/compiler/src/main/antlr3/org/apache/royale/compiler/internal/css/CSSTree.g b/compiler/src/main/antlr3/org/apache/royale/compiler/internal/css/CSSTree.g index cbb545d49..a7cf91245 100644 --- a/compiler/src/main/antlr3/org/apache/royale/compiler/internal/css/CSSTree.g +++ b/compiler/src/main/antlr3/org/apache/royale/compiler/internal/css/CSSTree.g @@ -131,7 +131,14 @@ namespaceStatement { final CSSNamespaceDefinition ns = new CSSNamespaceDefinition( $id.text, $uri.text, $start, tokenStream); - $stylesheet::namespaces.add(ns); + if (ns.getProblems().size() == 0) + { + $stylesheet::namespaces.add(ns); + } + else + { + problems.addAll(ns.getProblems()); + } } : ^(AT_NAMESPACE id=ID? uri=STRING) ; @@ -184,8 +191,20 @@ mediumCondition fontFace @after { - final CSSFontFace fontFace = new CSSFontFace($d.properties, $start, tokenStream); - $stylesheet::fontFaces.add(fontFace); + List<CSSProperty> properties = $d.properties; + if (properties == null) + { + properties = new ArrayList<CSSProperty>(); + } + final CSSFontFace fontFace = new CSSFontFace(properties, $start, tokenStream); + if (fontFace.getProblems().size() == 0) + { + $stylesheet::fontFaces.add(fontFace); + } + else + { + problems.addAll(fontFace.getProblems()); + } } : ^(AT_FONT_FACE d=declarationsBlock) ; diff --git a/compiler/src/main/java/org/apache/royale/compiler/internal/css/CSSFontFace.java b/compiler/src/main/java/org/apache/royale/compiler/internal/css/CSSFontFace.java index 169aa7067..296cfa2b2 100644 --- a/compiler/src/main/java/org/apache/royale/compiler/internal/css/CSSFontFace.java +++ b/compiler/src/main/java/org/apache/royale/compiler/internal/css/CSSFontFace.java @@ -19,18 +19,19 @@ package org.apache.royale.compiler.internal.css; -import static com.google.common.base.Preconditions.checkNotNull; - import java.util.ArrayList; import java.util.List; import org.antlr.runtime.TokenStream; import org.antlr.runtime.tree.CommonTree; - +import org.apache.royale.compiler.common.ISourceLocation; +import org.apache.royale.compiler.common.SourceLocation; import org.apache.royale.compiler.css.FontFaceSourceType; import org.apache.royale.compiler.css.ICSSFontFace; import org.apache.royale.compiler.css.ICSSProperty; import org.apache.royale.compiler.css.ICSSPropertyValue; +import org.apache.royale.compiler.problems.CSSRequiredDescriptorProblem; +import org.apache.royale.compiler.problems.ICompilerProblem; /** * Implementation for {@code @font-face} statement DOM. @@ -96,14 +97,30 @@ public class CSSFontFace extends CSSNodeBase implements ICSSFontFace } } - checkNotNull(srcValue, "'src' is required in @font-face"); - if (srcValue instanceof CSSArrayPropertyValue) - source = (CSSFunctionCallPropertyValue)(srcValue).getNthChild(0); + if (srcValue == null) + { + source = null; + ISourceLocation sourceLocation = new SourceLocation(tokenStream.getSourceName(), UNKNOWN, UNKNOWN, tree.getLine(), tree.getCharPositionInLine()); + problems.add(new CSSRequiredDescriptorProblem(sourceLocation, "@font-face", "src")); + } else - source = (CSSFunctionCallPropertyValue)srcValue; + { + if (srcValue instanceof CSSArrayPropertyValue) + source = (CSSFunctionCallPropertyValue)(srcValue).getNthChild(0); + else + source = (CSSFunctionCallPropertyValue)srcValue; + } - checkNotNull(fontFamilyValue, "'fontFamily' is required in @font-face"); - fontFamily = fontFamilyValue.toString(); + if (fontFamilyValue == null) + { + fontFamily = null; + ISourceLocation sourceLocation = new SourceLocation(tokenStream.getSourceName(), UNKNOWN, UNKNOWN, tree.getLine(), tree.getCharPositionInLine()); + problems.add(new CSSRequiredDescriptorProblem(sourceLocation, "@font-face", "fontFamily")); + } + else + { + fontFamily = fontFamilyValue.toString(); + } fontStyle = fontStyleValue == null ? "normal" : fontStyleValue.toString(); fontWeight = fontWeightValue == null ? "normal" : fontWeightValue.toString(); @@ -120,6 +137,13 @@ public class CSSFontFace extends CSSNodeBase implements ICSSFontFace private final boolean isEmbedAsCFF; private final boolean isAdvancedAntiAliasing; + private final List<ICompilerProblem> problems = new ArrayList<ICompilerProblem>(); + + public List<ICompilerProblem> getProblems() + { + return problems; + } + public ArrayList<ICSSPropertyValue> getSources() { return sources; diff --git a/compiler/src/main/java/org/apache/royale/compiler/internal/css/CSSNamespaceDefinition.java b/compiler/src/main/java/org/apache/royale/compiler/internal/css/CSSNamespaceDefinition.java index 2a2951adc..9eeee319f 100644 --- a/compiler/src/main/java/org/apache/royale/compiler/internal/css/CSSNamespaceDefinition.java +++ b/compiler/src/main/java/org/apache/royale/compiler/internal/css/CSSNamespaceDefinition.java @@ -19,12 +19,17 @@ package org.apache.royale.compiler.internal.css; -import static com.google.common.base.Preconditions.checkState; +import java.util.ArrayList; +import java.util.List; import org.antlr.runtime.TokenStream; import org.antlr.runtime.tree.CommonTree; - +import org.apache.royale.compiler.common.ISourceLocation; +import org.apache.royale.compiler.common.SourceLocation; import org.apache.royale.compiler.css.ICSSNamespaceDefinition; +import org.apache.royale.compiler.problems.CSSNamespaceEmptyURIProblem; +import org.apache.royale.compiler.problems.ICompilerProblem; + import com.google.common.base.Joiner; /** @@ -48,13 +53,32 @@ public class CSSNamespaceDefinition extends CSSNodeBase implements ICSSNamespace } this.prefix = prefix; - this.uri = CSSStringPropertyValue.stripQuotes(uri); - checkState(this.uri.length() > 0, "@namespace URI can't be empty."); + if (uri == null) + { + this.uri = ""; + } + else + { + this.uri = CSSStringPropertyValue.stripQuotes(uri); + } + + if (this.uri.length() == 0) + { + ISourceLocation sourceLocation = new SourceLocation(tokenStream.getSourceName(), UNKNOWN, UNKNOWN, tree.getLine(), tree.getCharPositionInLine()); + problems.add(new CSSNamespaceEmptyURIProblem(sourceLocation)); + } } private final String prefix; private final String uri; + private final List<ICompilerProblem> problems = new ArrayList<ICompilerProblem>(); + + public List<ICompilerProblem> getProblems() + { + return problems; + } + @Override public String getPrefix() { diff --git a/compiler/src/main/java/org/apache/royale/compiler/problems/CSSNamespaceEmptyURIProblem.java b/compiler/src/main/java/org/apache/royale/compiler/problems/CSSNamespaceEmptyURIProblem.java new file mode 100644 index 000000000..554329815 --- /dev/null +++ b/compiler/src/main/java/org/apache/royale/compiler/problems/CSSNamespaceEmptyURIProblem.java @@ -0,0 +1,37 @@ +/* + * + * 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; + +/** + * Problem generated when a CSS @namespace at-rule has an empty URI. + */ +public final class CSSNamespaceEmptyURIProblem extends CSSProblem +{ + public static final String DESCRIPTION = + "The '@namespace' rule requires an URI that is not empty."; + + public CSSNamespaceEmptyURIProblem(ISourceLocation location) + { + super(location); + } +} + \ No newline at end of file diff --git a/compiler/src/main/java/org/apache/royale/compiler/problems/CSSRequiredDescriptorProblem.java b/compiler/src/main/java/org/apache/royale/compiler/problems/CSSRequiredDescriptorProblem.java new file mode 100644 index 000000000..7dc4ab8c5 --- /dev/null +++ b/compiler/src/main/java/org/apache/royale/compiler/problems/CSSRequiredDescriptorProblem.java @@ -0,0 +1,42 @@ +/* + * + * 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; + +/** + * Problem generated when a CSS at-rule lacks a required descriptor. + */ +public final class CSSRequiredDescriptorProblem extends CSSProblem +{ + public static final String DESCRIPTION = + "The '${atRule}' rule requires a '${descriptorName}' descriptor."; + + public CSSRequiredDescriptorProblem(ISourceLocation location, String atRule, String descriptorName) + { + super(location); + this.atRule = atRule; + this.descriptorName = descriptorName; + } + + public final String atRule; + public final String descriptorName; +} + \ No newline at end of file diff --git a/compiler/src/test/java/org/apache/royale/compiler/internal/css/CSSNamespaceDefinitionTests.java b/compiler/src/test/java/org/apache/royale/compiler/internal/css/CSSNamespaceDefinitionTests.java index 7eb898dab..890dfdc7a 100644 --- a/compiler/src/test/java/org/apache/royale/compiler/internal/css/CSSNamespaceDefinitionTests.java +++ b/compiler/src/test/java/org/apache/royale/compiler/internal/css/CSSNamespaceDefinitionTests.java @@ -61,16 +61,10 @@ public class CSSNamespaceDefinitionTests extends CSSBaseTests { String code = " @namespace s ;"; - errorFilters = new String[1]; - errorFilters[0] = "missing STRING at ';'"; List<ICSSNamespaceDefinition> namespaces = getCSSNamespaceDefinition(code); - assertThat("namespaces.size()" , namespaces.size(), is(2) ); - - CSSNamespaceDefinition namespace = (CSSNamespaceDefinition) namespaces.get(1); - assertThat("namespace.getOperator()" , namespace.getOperator(), is( CSSModelTreeType.NAMESPACE_DEFINITION ) ); - assertThat("namespace.getPrefix()" , namespace.getPrefix(), is( "s" ) ); - //TODO why is it "missing STRING"? - assertThat("namespace.getURI()" , namespace.getURI(), is( "missing STRING" ) ); + // invalid CSS syntax, so there are no namespaces created, and an error + // is reported instead + assertThat("namespaces.size()" , namespaces.size(), is(0) ); } @Test @@ -78,17 +72,38 @@ public class CSSNamespaceDefinitionTests extends CSSBaseTests { { String code = " @namespace ;"; - - errorFilters = new String[1]; - errorFilters[0] = "missing STRING at ';'"; + List<ICSSNamespaceDefinition> namespaces = getCSSNamespaceDefinition(code); - assertThat("namespaces.size()" , namespaces.size(), is(2) ); + // invalid CSS syntax, so there are no namespaces created, and an error + // is reported instead + assertThat("namespaces.size()" , namespaces.size(), is(0) ); + } + + @Test + public void CSSNamespaceDefinitionTests_namespace3() + { + String code = + " @namespace \"\";"; + + List<ICSSNamespaceDefinition> namespaces = getCSSNamespaceDefinition(code); + // CSS syntax is valid, but empty namespace is not allowed, so it is + // ignored, except to report an error. + // only the "custom" namespace is recognized. + assertThat("namespaces.size()" , namespaces.size(), is(1) ); + } + + @Test + public void CSSNamespaceDefinitionTests_namespace4() + { + String code = + " @namespace pr \"\";"; + + List<ICSSNamespaceDefinition> namespaces = getCSSNamespaceDefinition(code); + // CSS syntax is valid, but empty namespace is not allowed, so it is + // ignored, except to report an error. + // only the "custom" namespace is recognized. + assertThat("namespaces.size()" , namespaces.size(), is(1) ); - CSSNamespaceDefinition namespace = (CSSNamespaceDefinition) namespaces.get(1); - assertThat("namespace.getOperator()" , namespace.getOperator(), is( CSSModelTreeType.NAMESPACE_DEFINITION ) ); - assertThat("namespace.getPrefix()" , namespace.getPrefix(), is( (String)null ) ); - //TODO why is it "missing STRING"? - assertThat("namespace.getURI()" , namespace.getURI(), is( "missing STRING" ) ); } @Test
