From: Samuel Schneider Date: Tue, 8 Mar 2022 12:56:24 +0000 (+0100) Subject: Add a knob to control warnings about unkeyed lists X-Git-Tag: v8.0.3~1 X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?p=yangtools.git;a=commitdiff_plain;h=303d81ba251f3b861c99595bb9562bcb348167d8 Add a knob to control warnings about unkeyed lists Warnings about unkeyed lists with config true are now can now be enabled (default) or disabled as needed. Also upgrades commons-cli version to 1.5.0: https://commons.apache.org/proper/commons-cli/changes-report.html#a1.5.0 JIRA: YANGTOOLS-1397 Change-Id: I8d576b18098421f16a83eb3c31d8c3a80966c4cb Signed-off-by: Samuel Schneider Signed-off-by: Robert Varga --- diff --git a/parser/yang-parser-api/src/main/java/org/opendaylight/yangtools/yang/parser/api/YangParserConfiguration.java b/parser/yang-parser-api/src/main/java/org/opendaylight/yangtools/yang/parser/api/YangParserConfiguration.java index 3514e31293..3343407dd1 100644 --- a/parser/yang-parser-api/src/main/java/org/opendaylight/yangtools/yang/parser/api/YangParserConfiguration.java +++ b/parser/yang-parser-api/src/main/java/org/opendaylight/yangtools/yang/parser/api/YangParserConfiguration.java @@ -16,6 +16,8 @@ import org.eclipse.jdt.annotation.NonNullByDefault; import org.eclipse.jdt.annotation.Nullable; import org.opendaylight.yangtools.concepts.Immutable; import org.opendaylight.yangtools.concepts.Mutable; +import org.opendaylight.yangtools.yang.model.api.meta.DeclarationReference; +import org.opendaylight.yangtools.yang.model.api.meta.DeclaredStatement; /** * A configuration of {@link YangParser} wiring for use with {@link YangParserFactory}. @@ -29,11 +31,13 @@ public final class YangParserConfiguration implements Immutable { private final ImportResolutionMode importResolutionMode; private final boolean retainDeclarationReferences; + private final boolean warnForUnkeyedLists; private YangParserConfiguration(final ImportResolutionMode importResolutionMode, - final boolean retainDeclarationReferences) { + final boolean retainDeclarationReferences, final boolean warnForUnkeyedLists) { this.importResolutionMode = requireNonNull(importResolutionMode); this.retainDeclarationReferences = retainDeclarationReferences; + this.warnForUnkeyedLists = warnForUnkeyedLists; } @Beta @@ -41,10 +45,28 @@ public final class YangParserConfiguration implements Immutable { return importResolutionMode; } + /** + * Return {@code true} if {@link DeclarationReference} to source location in the final parser product, notably + * making {@link DeclaredStatement#declarationReference()} available. + * + * @return {@code true} if declaration references should be retained + */ public boolean retainDeclarationReferences() { return retainDeclarationReferences; } + /** + * Issue a warning when a {@code list} statement without a {@code key} statement is found in the + * {@code config true} part of the schema tree. Such statements run contrary to + * RFC7950, but are readily supported + * by OpenDaylight infrastructure. + * + * @return {@code true} if non-compliant {@code list} statements should be reported + */ + public boolean warnForUnkeyedLists() { + return warnForUnkeyedLists; + } + @Override public int hashCode() { return Objects.hash(importResolutionMode, retainDeclarationReferences); @@ -71,13 +93,20 @@ public final class YangParserConfiguration implements Immutable { .toString(); } + /** + * Return a new {@link Builder} initialized to default configuration. + * + * @return A new builder + */ public static Builder builder() { return new Builder(); } public static final class Builder implements Mutable { private ImportResolutionMode importResolutionMode = ImportResolutionMode.DEFAULT; - private boolean retainDeclarationReferences = false; + private boolean retainDeclarationReferences; + // FIXME: YANGTOOLS-1423: default to false + private boolean warnForUnkeyedLists = true; private Builder() { // Hidden on purpose @@ -89,7 +118,7 @@ public final class YangParserConfiguration implements Immutable { * @return A YangParserConfiguration */ public YangParserConfiguration build() { - return new YangParserConfiguration(importResolutionMode, retainDeclarationReferences); + return new YangParserConfiguration(importResolutionMode, retainDeclarationReferences, warnForUnkeyedLists); } @Beta @@ -98,9 +127,38 @@ public final class YangParserConfiguration implements Immutable { return this; } + /** + * Retain {@link DeclarationReference} to source location in the final parser product. This option results in + * quite significant memory overhead for storage of {@link DeclaredStatement}, but makes + * {@link DeclaredStatement#declarationReference()} available, which is useful in certain scenarios, for example + * YANG editors. + * + *

+ * This option is disabled by default. + * + * @param newRetainDeclarationReferences {@code true} if declaration references should be retained + * @return This builder + */ public Builder retainDeclarationReferences(final boolean newRetainDeclarationReferences) { retainDeclarationReferences = newRetainDeclarationReferences; return this; } + + /** + * Issue a warning when a {@code list} statement without a {@code key} statement is found in the + * {@code config true} part of the schema tree. Such statements run contrary to + * RFC7950, but are readily supported + * by OpenDaylight infrastructure. + * + *

+ * This option is enabled by default. + * + * @param newWarnForUnkeyedLists {@code true} if non-compliant {@code list} statements should be reported + * @return This builder + */ + public Builder warnForUnkeyedLists(final boolean newWarnForUnkeyedLists) { + warnForUnkeyedLists = newWarnForUnkeyedLists; + return this; + } } } diff --git a/parser/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/list/ListStatementSupport.java b/parser/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/list/ListStatementSupport.java index 189e695bad..5c25cbf675 100644 --- a/parser/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/list/ListStatementSupport.java +++ b/parser/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/list/ListStatementSupport.java @@ -114,8 +114,11 @@ public final class ListStatementSupport .addOptional(YangStmtMapping.WHEN) .build(); + private final boolean warnForUnkeyedLists; + ListStatementSupport(final YangParserConfiguration config, final SubstatementValidator validator) { super(YangStmtMapping.LIST, instantiatedPolicy(), config, validator); + this.warnForUnkeyedLists = config.warnForUnkeyedLists(); } public static @NonNull ListStatementSupport rfc6020Instance(final YangParserConfiguration config) { @@ -171,7 +174,8 @@ public final class ListStatementSupport } final int flags = computeFlags(stmt, substatements); - if (stmt.effectiveConfig() == EffectiveConfig.TRUE && keyDefinition.isEmpty() && isInstantied(stmt)) { + if (warnForUnkeyedLists && stmt.effectiveConfig() == EffectiveConfig.TRUE + && keyDefinition.isEmpty() && isInstantied(stmt)) { warnConfigList(stmt); } diff --git a/tools/yang-model-validator/pom.xml b/tools/yang-model-validator/pom.xml index a0bc8d222d..3a8dac24b1 100644 --- a/tools/yang-model-validator/pom.xml +++ b/tools/yang-model-validator/pom.xml @@ -42,7 +42,7 @@ commons-cli commons-cli - 1.4 + 1.5.0 diff --git a/tools/yang-model-validator/src/main/java/org/opendaylight/yangtools/yang/validator/Main.java b/tools/yang-model-validator/src/main/java/org/opendaylight/yangtools/yang/validator/Main.java index 6ccae443f7..858d7b9e22 100644 --- a/tools/yang-model-validator/src/main/java/org/opendaylight/yangtools/yang/validator/Main.java +++ b/tools/yang-model-validator/src/main/java/org/opendaylight/yangtools/yang/validator/Main.java @@ -49,6 +49,8 @@ import org.slf4j.LoggerFactory; * -v, --verbose shows details about the results of test running. * -o, --output path to output file for logs. Output file will be overwritten. * -m, --module-name validate yang by module name. + * -wul, --warning-for-unkeyed-lists + * add warnings about unkeyed lists with config true. */ @SuppressWarnings({"checkstyle:LoggerMustBeSlf4j", "checkstyle:LoggerFactoryClassParameter"}) public final class Main { @@ -76,27 +78,25 @@ public final class Main { private static final Option QUIET = new Option("q", "quiet", false, "completely suppress output."); private static final Option VERBOSE = new Option("v", "verbose", false, "shows details about the results of test running."); + private static final Option LIST_WARNING_ON = new Option("wul", "warning-for-unkeyed-lists", false, + "add warnings about unkeyed lists with config true"); + private static final Option LIST_WARNING_OFF = new Option("no-wul", "no-warning-for-unkeyed-lists", false, + "do not add warnings about unkeyed lists with config true"); private Main() { // Hidden on purpose } private static Options createOptions() { - final Options options = new Options(); - options.addOption(HELP); - options.addOption(PATH); - options.addOption(RECURSIVE); - - final OptionGroup verbosity = new OptionGroup(); - verbosity.addOption(DEBUG); - verbosity.addOption(QUIET); - verbosity.addOption(VERBOSE); - options.addOptionGroup(verbosity); - - options.addOption(OUTPUT); - options.addOption(MODULE_NAME); - options.addOption(FEATURE); - return options; + return new Options() + .addOption(HELP) + .addOption(PATH) + .addOption(RECURSIVE) + .addOptionGroup(new OptionGroup().addOption(DEBUG).addOption(QUIET).addOption(VERBOSE)) + .addOptionGroup(new OptionGroup().addOption(LIST_WARNING_ON).addOption(LIST_WARNING_OFF)) + .addOption(OUTPUT) + .addOption(MODULE_NAME) + .addOption(FEATURE); } public static void main(final String[] args) { @@ -123,6 +123,13 @@ public final class Main { LOG_ROOT.detachAndStopAllAppenders(); } + final boolean warnForUnkeyedLists; + if (arguments.hasOption(LIST_WARNING_ON.getLongOpt()) || !arguments.hasOption(LIST_WARNING_OFF.getLongOpt())) { + warnForUnkeyedLists = true; + } else { + warnForUnkeyedLists = false; + } + final List yangLibDirs = initYangDirsPath(arguments); final List yangFiles = new ArrayList<>(); final String[] moduleNameValues = arguments.getOptionValues(MODULE_NAME.getLongOpt()); @@ -133,7 +140,8 @@ public final class Main { final Set supportedFeatures = initSupportedFeatures(arguments); - runSystemTest(yangLibDirs, yangFiles, supportedFeatures, arguments.hasOption("recursive")); + runSystemTest(yangLibDirs, yangFiles, supportedFeatures, arguments.hasOption(RECURSIVE.getLongOpt()), + warnForUnkeyedLists); LOG_ROOT.getLoggerContext().reset(); } @@ -163,7 +171,7 @@ public final class Main { @SuppressFBWarnings({ "DM_EXIT", "DM_GC" }) @SuppressWarnings("checkstyle:illegalCatch") private static void runSystemTest(final List yangLibDirs, final List yangFiles, - final Set supportedFeatures, final boolean recursiveSearch) { + final Set supportedFeatures, final boolean recursiveSearch, final boolean warnForUnkeyedLists) { LOG.info("Yang model dirs: {} ", yangLibDirs); LOG.info("Yang model files: {} ", yangFiles); LOG.info("Supported features: {} ", supportedFeatures); @@ -174,7 +182,8 @@ public final class Main { final Stopwatch stopWatch = Stopwatch.createStarted(); try { - context = SystemTestUtils.parseYangSources(yangLibDirs, yangFiles, supportedFeatures, recursiveSearch); + context = SystemTestUtils.parseYangSources(yangLibDirs, yangFiles, supportedFeatures, + recursiveSearch, warnForUnkeyedLists); } catch (final Exception e) { LOG.error("Failed to create SchemaContext.", e); System.exit(1); diff --git a/tools/yang-model-validator/src/main/java/org/opendaylight/yangtools/yang/validator/SystemTestUtils.java b/tools/yang-model-validator/src/main/java/org/opendaylight/yangtools/yang/validator/SystemTestUtils.java index d626b6d7d8..db42013067 100644 --- a/tools/yang-model-validator/src/main/java/org/opendaylight/yangtools/yang/validator/SystemTestUtils.java +++ b/tools/yang-model-validator/src/main/java/org/opendaylight/yangtools/yang/validator/SystemTestUtils.java @@ -34,6 +34,7 @@ import org.opendaylight.yangtools.yang.common.YangConstants; import org.opendaylight.yangtools.yang.model.api.EffectiveModelContext; import org.opendaylight.yangtools.yang.model.repo.api.YangTextSchemaSource; import org.opendaylight.yangtools.yang.parser.api.YangParser; +import org.opendaylight.yangtools.yang.parser.api.YangParserConfiguration; import org.opendaylight.yangtools.yang.parser.api.YangParserException; import org.opendaylight.yangtools.yang.parser.api.YangParserFactory; @@ -61,7 +62,8 @@ final class SystemTestUtils { }; static EffectiveModelContext parseYangSources(final List yangLibDirs, final List yangTestFiles, - final Set supportedFeatures, final boolean recursiveSearch) throws IOException, YangParserException { + final Set supportedFeatures, final boolean recursiveSearch, + final boolean warnForUnkeyedLists) throws IOException, YangParserException { /* * Current dir "." should be always present implicitly in the list of * directories where dependencies are searched for @@ -84,14 +86,16 @@ final class SystemTestUtils { } } - return parseYangSources(supportedFeatures, testFiles, libFiles); + return parseYangSources(supportedFeatures, testFiles, libFiles, warnForUnkeyedLists); } static EffectiveModelContext parseYangSources(final Set supportedFeatures, final List testFiles, - final List libFiles) throws IOException, YangParserException { + final List libFiles, final boolean warnForUnkeyedLists) throws IOException, YangParserException { checkArgument(!testFiles.isEmpty(), "No yang sources"); - final YangParser parser = PARSER_FACTORY.createParser(); + final YangParserConfiguration configuration = YangParserConfiguration.builder() + .warnForUnkeyedLists(warnForUnkeyedLists).build(); + final YangParser parser = PARSER_FACTORY.createParser(configuration); if (supportedFeatures != null) { parser.setSupportedFeatures(supportedFeatures); }