Refactor ArgumentContextUtils 81/87781/1
authorRobert Varga <robert.varga@pantheon.tech>
Wed, 12 Feb 2020 21:21:19 +0000 (22:21 +0100)
committerRobert Varga <robert.varga@pantheon.tech>
Sun, 16 Feb 2020 09:50:36 +0000 (10:50 +0100)
This is a static utility class, whose behavior depends on YANG
version. Refactor it into an enum, which expresses the differences
in terms of two separate subclasses, so that JIT can make the right
decisions.

Also mark spots for follow-up improvements.

JIRA: YANGTOOLS-1079
Change-Id: I1bc1dad7c0a313065d32174578ccbe941a9f3e3a
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>
yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/repo/ArgumentContextUtils.java
yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/repo/StatementContextVisitor.java
yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/repo/YangModelDependencyInfo.java

index 3f29ed9121605af420e23958b1505c1114011c1a..c104afde638027d40f50496344ec55454588d5a2 100644 (file)
@@ -9,16 +9,66 @@ package org.opendaylight.yangtools.yang.parser.rfc7950.repo;
 
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.CharMatcher;
-import java.util.Collections;
 import java.util.List;
 import java.util.regex.Pattern;
 import org.antlr.v4.runtime.tree.TerminalNode;
+import org.eclipse.jdt.annotation.NonNull;
 import org.opendaylight.yangtools.antlrv4.code.gen.YangStatementParser.ArgumentContext;
 import org.opendaylight.yangtools.yang.common.YangVersion;
 import org.opendaylight.yangtools.yang.parser.spi.source.SourceException;
 import org.opendaylight.yangtools.yang.parser.spi.source.StatementSourceReference;
 
-final class ArgumentContextUtils {
+/**
+ * Utilities for dealing with YANG statement argument strings, encapsulated in ANTLR grammar's ArgumentContext.
+ */
+enum ArgumentContextUtils {
+    /**
+     * YANG 1.0 version of strings, which were not completely clarified in RFC6020.
+     */
+    RFC6020 {
+        @Override
+        void checkDoubleQuotedString(final String str, final StatementSourceReference ref) {
+            // No-op
+        }
+
+        @Override
+        void checkUnquotedString(final String str, final StatementSourceReference ref) {
+            // No-op
+        }
+    },
+    /**
+     * YANG 1.1 version of strings, which were clarified in RFC7950.
+     */
+    // NOTE: the differences clarified lead to a proper ability to delegate this to ANTLR lexer, but that does not
+    //       understand versions and needs to work with both.
+    RFC7950 {
+        @Override
+        void checkDoubleQuotedString(final String str, final StatementSourceReference ref) {
+            for (int i = 0; i < str.length() - 1; i++) {
+                if (str.charAt(i) == '\\') {
+                    switch (str.charAt(i + 1)) {
+                        case 'n':
+                        case 't':
+                        case '\\':
+                        case '\"':
+                            i++;
+                            break;
+                        default:
+                            throw new SourceException(ref, "YANG 1.1: illegal double quoted string (%s). In double "
+                                    + "quoted string the backslash must be followed by one of the following character "
+                                    + "[n,t,\",\\], but was '%s'.", str, str.charAt(i + 1));
+                    }
+                }
+            }
+        }
+
+        @Override
+        void checkUnquotedString(final String str, final StatementSourceReference ref) {
+            SourceException.throwIf(ANYQUOTE_MATCHER.matchesAnyOf(str), ref,
+                "YANG 1.1: unquoted string (%s) contains illegal characters", str);
+        }
+    };
+
     private static final CharMatcher WHITESPACE_MATCHER = CharMatcher.whitespace();
     private static final CharMatcher ANYQUOTE_MATCHER = CharMatcher.anyOf("'\"");
     private static final Pattern ESCAPED_DQUOT = Pattern.compile("\\\"", Pattern.LITERAL);
@@ -26,50 +76,74 @@ final class ArgumentContextUtils {
     private static final Pattern ESCAPED_LF = Pattern.compile("\\n", Pattern.LITERAL);
     private static final Pattern ESCAPED_TAB = Pattern.compile("\\t", Pattern.LITERAL);
 
-    private ArgumentContextUtils() {
-        // Hidden on purpose
+    static @NonNull ArgumentContextUtils forVersion(final YangVersion version) {
+        switch (version) {
+            case VERSION_1:
+                return RFC6020;
+            case VERSION_1_1:
+                return RFC7950;
+            default:
+                throw new IllegalStateException("Unhandled version " + version);
+        }
     }
 
-    static String stringFromStringContext(final ArgumentContext context, final YangVersion yangVersion,
-            final StatementSourceReference ref) {
+    final @NonNull String stringFromStringContext(final ArgumentContext context, final StatementSourceReference ref) {
         final StringBuilder sb = new StringBuilder();
-        List<TerminalNode> strings = context.STRING();
-        if (strings.isEmpty()) {
-            strings = Collections.singletonList(context.IDENTIFIER());
-        }
-        for (final TerminalNode stringNode : strings) {
-            final String str = stringNode.getText();
-            final char firstChar = str.charAt(0);
-            final char lastChar = str.charAt(str.length() - 1);
-            if (firstChar == '"' && lastChar == '"') {
-                final String innerStr = str.substring(1, str.length() - 1);
-                /*
-                 * Unescape escaped double quotes, tabs, new line and backslash
-                 * in the inner string and trim the result.
-                 */
-                checkDoubleQuotedString(innerStr, yangVersion, ref);
-                sb.append(unescape(trimWhitespace(innerStr, stringNode.getSymbol().getCharPositionInLine())));
-            } else if (firstChar == '\'' && lastChar == '\'') {
-                /*
-                 * According to RFC6020 a single quote character cannot occur in
-                 * a single-quoted string, even when preceded by a backslash.
-                 */
-                sb.append(str, 1, str.length() - 1);
-            } else {
-                checkUnquotedString(str, yangVersion, ref);
-                sb.append(str);
+        final List<TerminalNode> strings = context.STRING();
+        if (!strings.isEmpty()) {
+            for (final TerminalNode stringNode : strings) {
+                appendString(sb, stringNode, ref);
             }
+        } else {
+            appendString(sb, context.IDENTIFIER(), ref);
         }
+
         return sb.toString();
     }
 
+    private void appendString(final StringBuilder sb, final TerminalNode stringNode,
+            final StatementSourceReference ref) {
+
+        final String str = stringNode.getText();
+        final char firstChar = str.charAt(0);
+        final char lastChar = str.charAt(str.length() - 1);
+        // NOTE: Enforcement and transformation logic here should certainly be pushed down to the lexer, as ANTLR can
+        //       account the for it with lexer modes. One problem is that lexing here depends on version being lexed,
+        //       hence we really would have to re-parse the YANG file after determining its version. We certainly do not
+        //       want to do that.
+        // FIXME: YANGTOOLS-1079: but since we are performing quoting checks, perhaps at least that part could be lexed?
+        if (firstChar == '"' && lastChar == '"') {
+            final String innerStr = str.substring(1, str.length() - 1);
+            /*
+             * Unescape escaped double quotes, tabs, new line and backslash
+             * in the inner string and trim the result.
+             */
+            checkDoubleQuotedString(innerStr, ref);
+            sb.append(unescape(trimWhitespace(innerStr, stringNode.getSymbol().getCharPositionInLine())));
+        } else if (firstChar == '\'' && lastChar == '\'') {
+            /*
+             * According to RFC6020 a single quote character cannot occur in
+             * a single-quoted string, even when preceded by a backslash.
+             */
+            sb.append(str, 1, str.length() - 1);
+        } else {
+            checkUnquotedString(str, ref);
+            sb.append(str);
+        }
+    }
+
+    abstract void checkDoubleQuotedString(String str, StatementSourceReference ref);
+
+    abstract void checkUnquotedString(String str, StatementSourceReference ref);
+
     private static String unescape(final String str) {
         final int backslash = str.indexOf('\\');
         if (backslash == -1) {
             return str;
         }
 
-        // FIXME: given we the leading backslash, it would be more efficient to walk the string and unescape in one go
+        // FIXME: YANGTOOLS-1079: given we the leading backslash, it would be more efficient to walk the string and
+        //                        unescape in one go
         return ESCAPED_TAB.matcher(
                     ESCAPED_LF.matcher(
                         ESCAPED_BACKSLASH.matcher(
@@ -79,36 +153,6 @@ final class ArgumentContextUtils {
                .replaceAll("\\\t");
     }
 
-    private static void checkUnquotedString(final String str, final YangVersion yangVersion,
-            final StatementSourceReference ref) {
-        if (yangVersion == YangVersion.VERSION_1_1) {
-            SourceException.throwIf(ANYQUOTE_MATCHER.matchesAnyOf(str), ref,
-                "YANG 1.1: unquoted string (%s) contains illegal characters", str);
-        }
-    }
-
-    private static void checkDoubleQuotedString(final String str, final YangVersion yangVersion,
-            final StatementSourceReference ref) {
-        if (yangVersion == YangVersion.VERSION_1_1) {
-            for (int i = 0; i < str.length() - 1; i++) {
-                if (str.charAt(i) == '\\') {
-                    switch (str.charAt(i + 1)) {
-                        case 'n':
-                        case 't':
-                        case '\\':
-                        case '\"':
-                            i++;
-                            break;
-                        default:
-                            throw new SourceException(ref, "YANG 1.1: illegal double quoted string (%s). In double "
-                                    + "quoted string the backslash must be followed by one of the following character "
-                                    + "[n,t,\",\\], but was '%s'.", str, str.charAt(i + 1));
-                    }
-                }
-            }
-        }
-    }
-
     @VisibleForTesting
     static String trimWhitespace(final String str, final int dquot) {
         int brk = str.indexOf('\n');
index 5672a36391fb6eedf3122661b5714f20e20fc27c..c1028ffc78930e31f54a6496f0bb837381d91bec 100644 (file)
@@ -30,8 +30,8 @@ import org.opendaylight.yangtools.yang.parser.spi.source.StatementWriter.Resumed
 
 class StatementContextVisitor {
     private final QNameToStatementDefinition stmtDef;
+    private final ArgumentContextUtils utils;
     private final StatementWriter writer;
-    private final YangVersion yangVersion;
     private final PrefixToModule prefixes;
     private final String sourceName;
 
@@ -39,7 +39,7 @@ class StatementContextVisitor {
             final QNameToStatementDefinition stmtDef, final PrefixToModule prefixes, final YangVersion yangVersion) {
         this.writer = requireNonNull(writer);
         this.stmtDef = requireNonNull(stmtDef);
-        this.yangVersion = requireNonNull(yangVersion);
+        this.utils = ArgumentContextUtils.forVersion(yangVersion);
         this.sourceName = sourceName;
         this.prefixes = prefixes;
     }
@@ -110,8 +110,7 @@ class StatementContextVisitor {
             }
 
             final ArgumentContext argumentCtx = ctx.getChild(ArgumentContext.class, 0);
-            final String argument = argumentCtx == null ? null
-                    : ArgumentContextUtils.stringFromStringContext(argumentCtx, yangVersion, ref);
+            final String argument = argumentCtx == null ? null : utils.stringFromStringContext(argumentCtx, ref);
             writer.startStatement(myOffset, def, argument, ref);
         }
 
index d06e15ea0612d06eefb52d04b4b15f458df46ffe..b360037273967dc6cbd32fdec35d4a0e8879e842 100644 (file)
@@ -29,7 +29,6 @@ import org.opendaylight.yangtools.concepts.SemVer;
 import org.opendaylight.yangtools.openconfig.model.api.OpenConfigStatements;
 import org.opendaylight.yangtools.yang.common.QName;
 import org.opendaylight.yangtools.yang.common.Revision;
-import org.opendaylight.yangtools.yang.common.YangVersion;
 import org.opendaylight.yangtools.yang.model.api.ModuleImport;
 import org.opendaylight.yangtools.yang.model.api.YangStmtMapping;
 import org.opendaylight.yangtools.yang.model.parser.api.YangSyntaxErrorException;
@@ -330,7 +329,8 @@ public abstract class YangModelDependencyInfo {
         final StatementSourceReference ref = getReference(source, stmt);
         final ArgumentContext arg = stmt.argument();
         checkArgument(arg != null, "Missing %s at %s", desc, ref);
-        return ArgumentContextUtils.stringFromStringContext(arg, YangVersion.VERSION_1, ref);
+        // TODO: we probably need to understand yang version first....
+        return ArgumentContextUtils.RFC6020.stringFromStringContext(arg, ref);
     }
 
     private static StatementSourceReference getReference(final SourceIdentifier source,