bugfix/423_uniform-tag-parsing

doc/changes/changes_4.11.0.md

@@ -19,7 +19,8 @@ The user guide documentation now has automatic lateral navigation.
 
 ## Bugfixes
 
-- #574: Fixed invalid HTML when inline code contains underscores
+* #423: Fixed tag parsing inconsistency in Markdown and reStructuredText importers
+* #574: Fixed invalid HTML when inline code contains underscores
 
 ## Documentation
 

doc/spec/design.md

@@ -1134,6 +1134,23 @@ Covers:
 
 Needs: impl, utest
 
+#### Markdown Tag Format
+`dsn~md.tags-format~1`
+
+The Markdown and reStructuredText Importers support defining tags for specification items using either a single-line comma-separated list or an itemized bullet list. During import, each declared tag is validated; invalid tags are rejected while all valid tags are imported.
+
+    tag = (ALPHA / DIGIT) *(ALPHA / DIGIT / "_")
+
+    tags-single-line = "Tags:" *WSP tag *("," *WSP tag)
+
+    tags-list = "Tags:" *(LINEBREAK *WSP ("*" / "+" / "-") *WSP tag)
+
+Covers:
+
+* `req~tag-format~1`
+
+Needs: impl, utest
+
 ### Coverage Tag Format
 
 #### Full Coverage Tag Format

doc/spec/system_requirements.md

@@ -332,6 +332,22 @@ Covers:
 
 Needs: dsn
 
+##### Tag Format
+`req~tag-format~1`
+
+Tags assigned to specification items in lightweight markup documents must have one or more characters. The first character of a tag must be an ASCII letter or digit. Subsequent characters may additionally contain underscores (`_`). When importing tag declarations (single-line comma-separated lists or itemized lists), invalid tags are rejected while valid tags are imported.
+
+Rationale:
+
+The tag format is easy to parse and robust. It is also in line with what most other programs would consider a valid tag, which is important for interoperability.
+
+Covers:
+
+* [feat~markdown-import~1](#markdown-import)
+* [feat~rst-import~1](#restructured-text-rst-import)
+
+Needs: dsn
+
 #### Markdown
 
 Markdown is a simple ASCII-based markup format that is designed to be human-readable in the source. While it can be rendered into HTML, it is perfectly eye-friendly even before rendering.

doc/user_guide/use_cases/distributing_the_detailing_work.md

@@ -35,4 +35,14 @@ This tells OFT to read all known specification files from the directory "import/
 
 If you want to also import specification items that do not have any tags, add a single underscore "_" as the first entry in the comma-separated list of tags:
 
-    oft convert -t _,AuthenticationProvider,ServiceDiscovery,MapProvider import/arch/ > arch_filtered_by_web_services.xml
+    oft convert -t _,AuthenticationProvider,ServiceDiscovery,MapProvider import/arch/ > arch_filtered_by_web_services.xml
+
+#### Tag Syntax and Formats
+
+Tags must start with an ASCII letter or digit and may contain letters, digits, and underscores (`_`). Tags can be defined either as a comma-separated list on a single line (`Tags: tag1, tag2`) or as an itemized bullet list:
+
+    Tags:
+    - AuthenticationProvider
+    - ServiceDiscovery
+
+When parsing tag declarations, individual invalid tags are ignored while valid tags are imported. For full syntax details, see [Writing a Specification](writing_a_specification.md#tags).

doc/user_guide/use_cases/writing_a_specification.md

@@ -175,7 +175,24 @@ is functionally equivalent to
 
 ##### `Tags`
 
-Tags are described in detail later in this document, see section [Distributing the Detailing Work](distributing_the_detailing_work.md#distributing-the-detailing-work).
+The `Tags` keyword assigns one or more tags to a specification item. Tags can be used to filter specification items during import and processing (see [Distributing the Detailing Work](distributing_the_detailing_work.md#distributing-the-detailing-work) and [Import options](../reference/oft_command_line.md#import-options)).
+
+Tags must consist of one or more characters. The first character must be an ASCII letter (`a-z`, `A-Z`) or digit (`0-9`), and subsequent characters may additionally contain underscores (`_`).
+
+`Tags` can be declared in two formats:
+
+**Variant a) Single-line comma-separated list**
+
+    Tags: AuthenticationProvider, ServiceDiscovery, MapProvider
+
+**Variant b) Itemized list**
+
+    Tags:
+    - AuthenticationProvider
+    - ServiceDiscovery
+    - MapProvider
+
+Bullet characters `*`, `+`, or `-` can be used for the itemized list. When importing tag declarations, any invalid or malformed tag entries are ignored, while all valid tags are imported.
 
 #### Notation in Multiline Text Blocks
 

importer/lightweightmarkup/src/main/java/org/itsallcode/openfasttrace/importer/lightweightmarkup/AbstractLightWeightMarkupImporter.java

@@ -1,5 +1,7 @@
 package org.itsallcode.openfasttrace.importer.lightweightmarkup;
 
+import java.util.regex.Pattern;
+
 import org.itsallcode.openfasttrace.api.core.*;
 import org.itsallcode.openfasttrace.api.importer.ImportEventListener;
 import org.itsallcode.openfasttrace.api.importer.Importer;
@@ -14,6 +16,9 @@
  */
 public abstract class AbstractLightWeightMarkupImporter implements Importer, LineReaderCallback
 {
+    @SuppressWarnings("java:S5867") // Intentionally only allow ASCII letters in tags.
+    private static final Pattern TAG_PATTERN = Pattern.compile("\\p{Alnum}\\w*");
+
     /** File to be imported */
     protected final InputFile file;
     /** Listener for import events */
@@ -288,15 +293,21 @@ private SourceRange range(final int start, final int end)
         return new SourceRange(new SourcePosition(line, start), new SourcePosition(line, end));
     }
 
+
     /**
      * Add one or more tags.
      */
+    // [impl->dsn~md.tags-format~1]
     protected void addTag()
     {
         final String tags = this.stateMachine.getLastToken();
         for (final String tag : tags.split(","))
         {
-            this.listener.addTag(tag.trim());
+            final String trimmed = tag.trim();
+            if (TAG_PATTERN.matcher(trimmed).matches())
+            {
+                this.listener.addTag(trimmed);
+            }
         }
     }
 

importer/lightweightmarkup/src/main/java/org/itsallcode/openfasttrace/importer/lightweightmarkup/PatternConstants.java

@@ -0,0 +1,30 @@
+package org.itsallcode.openfasttrace.importer.lightweightmarkup;
+
+import org.itsallcode.openfasttrace.api.core.SpecificationItemId;
+
+/**
+ * Common regular expression constants used across lightweight markup formats.
+ */
+public final class PatternConstants
+{
+    /** Matches valid artifact types (e.g., "req", "test"). */
+    public static final String ARTIFACT_TYPE = "\\p{Alpha}+";
+    /** Matches valid bullet points ("+", "*", "-"). */
+    public static final String BULLETS = "[+*-]";
+    // [impl->dsn~md.tags-format~1]
+    /** Matches valid tag names. */
+    public static final String TAG_PATTERN = "\\p{Aplha}\\w*";
+    /** Matches zero to three whitespace characters. */
+    public static final String UP_TO_3_WHITESPACES = "\\s{0,3}";
+    // [impl->dsn~md.requirement-references~1]
+    /** Matches a requirement reference after a bullet. Capture group 1 holds the ID. */
+    public static final String REFERENCE_AFTER_BULLET = UP_TO_3_WHITESPACES
+            + PatternConstants.BULLETS + "(?:.*\\W)?" //
+            + "(" + SpecificationItemId.ID_PATTERN + ")" //
+            + "(?:\\W.*)?";
+
+    private PatternConstants()
+    {
+        // not instantiable
+    }
+}

importer/lightweightmarkup/src/test/java/org/itsallcode/openfasttrace/importer/lightweightmarkup/PatternConstantsTest.java

@@ -0,0 +1,92 @@
+package org.itsallcode.openfasttrace.importer.lightweightmarkup;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.is;
+import static org.junit.jupiter.api.Assertions.assertAll;
+
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
+
+class PatternConstantsTest
+{
+    @ParameterizedTest
+    @ValueSource(strings = { "req", "feat", "DSN", "Artifact" })
+    void testArtifactTypeMatchesValid(final String input)
+    {
+        assertThat(input.matches(PatternConstants.ARTIFACT_TYPE), is(true));
+    }
+
+    @ParameterizedTest
+    @ValueSource(strings = { "123", "req_1", "feat-item", "" })
+    void testArtifactTypeRejectsInvalid(final String input)
+    {
+        assertThat(input.matches(PatternConstants.ARTIFACT_TYPE), is(false));
+    }
+
+    @ParameterizedTest
+    @ValueSource(strings = { "+", "*", "-" })
+    void testBulletsMatchesValid(final String input)
+    {
+        assertThat(input.matches(PatternConstants.BULLETS), is(true));
+    }
+
+    @ParameterizedTest
+    @ValueSource(strings = { "/", "#", "=", "++", "" })
+    void testBulletsRejectsInvalid(final String input)
+    {
+        assertThat(input.matches(PatternConstants.BULLETS), is(false));
+    }
+
+    // [utest->dsn~md.tags-format~1]
+    @ParameterizedTest
+    @ValueSource(strings = { "tag", "TAG_1", "0_valid", "a" })
+    void testTagPatternMatchesValid(final String input)
+    {
+        assertThat(input.matches(PatternConstants.TAG_PATTERN), is(true));
+    }
+
+    // [utest->dsn~md.tags-format~1]
+    @ParameterizedTest
+    @ValueSource(strings = { "_invalid", "-invalid", "tag with spaces", "" })
+    void testTagPatternRejectsInvalid(final String input)
+    {
+        assertThat(input.matches(PatternConstants.TAG_PATTERN), is(false));
+    }
+
+    @ParameterizedTest
+    @ValueSource(strings = { "", " ", "  ", "   " })
+    void testUpTo3WhitespacesMatchesValid(final String input)
+    {
+        assertThat(input.matches(PatternConstants.UP_TO_3_WHITESPACES), is(true));
+    }
+
+    @ParameterizedTest
+    @ValueSource(strings = { "    ", "\t\t\t\t" })
+    void testUpTo3WhitespacesRejectsMoreThan3(final String input)
+    {
+        assertThat(input.matches(PatternConstants.UP_TO_3_WHITESPACES), is(false));
+    }
+
+    @ParameterizedTest
+    @ValueSource(strings = { "* req~name~1", "  * req~name~1", "   * req~name~1", "+ req~name~1", "- req~name~1",
+            "* covers: req~name~1", "* req~name~1 extra" })
+    void testReferenceAfterBulletMatchesValid(final String input)
+    {
+        // Given/When/Then: input matches the pattern and captures the ID
+        final Matcher matcher = Pattern.compile(PatternConstants.REFERENCE_AFTER_BULLET).matcher(input);
+        assertAll(() -> assertThat(matcher.matches(), is(true)),
+                () -> assertThat(matcher.group(1), equalTo("req~name~1")));
+    }
+
+    @ParameterizedTest
+    @ValueSource(strings = { "    * req~name~1", "req~name~1", "*", "  *  " })
+    void testReferenceAfterBulletRejectsInvalid(final String input)
+    {
+        // Given/When/Then: input does not match
+        assertThat(Pattern.compile(PatternConstants.REFERENCE_AFTER_BULLET).matcher(input).matches(), is(false));
+    }
+}

importer/markdown/src/main/java/org/itsallcode/openfasttrace/importer/markdown/MarkdownImporter.java

@@ -159,6 +159,7 @@ protected Transition[] configureTransitions()
                 transition(NEEDS_LIST , TAGS       , MdPattern.TAGS       , () -> {}                           ),
                 transition(NEEDS_LIST , START      , MdPattern.FORWARD    , () -> {endItem(); forward();}      ),
 
+                // [impl->dsn~md.tags-format~1]
                 transition(TAGS       , TAGS       , MdPattern.TAG_ENTRY  , this::addTag                       ),
                 transition(TAGS       , SPEC_ITEM  , MdPattern.ID         , this::beginItem                    ),
                 transition(TAGS       , TITLE      , SECTION_TITLE        , () -> {endItem(); rememberTitle();}),

importer/markdown/src/main/java/org/itsallcode/openfasttrace/importer/markdown/MdPattern.java

@@ -2,6 +2,7 @@
 
 import org.itsallcode.openfasttrace.api.core.SpecificationItemId;
 import org.itsallcode.openfasttrace.importer.lightweightmarkup.ForwardingSpecificationItem;
+import org.itsallcode.openfasttrace.importer.lightweightmarkup.PatternConstants;
 import org.itsallcode.openfasttrace.importer.lightweightmarkup.statemachine.LinePattern;
 import org.itsallcode.openfasttrace.importer.lightweightmarkup.statemachine.SimpleLinePattern;
 
@@ -13,6 +14,7 @@ enum MdPattern
 {
     // [impl->dsn~md.specification-item-title~1]
     // [impl->dsn~md.artifact-forwarding-notation~1]
+    // [impl->dsn~md.tags-format~1]
 
     // @formatter:off
     CODE_BEGIN(" *[`~]{3,30}\\w*\\s*"),
@@ -49,11 +51,11 @@ enum MdPattern
     NOT_EMPTY("([^\n\r]+)"),
     RATIONALE("Rationale:\\s*"),
     STATUS("Status:\\s*(approved|proposed|draft|rejected)\\s*"),
-    TAGS_INT("Tags:(\\s*\\w+\\s*(?:,\\s*\\w+\\s*)*)"),
+    TAGS_INT("Tags:\\s*(\\S.*)"),
     TAGS("Tags:\\s*"),
     TAG_ENTRY(PatternConstants.UP_TO_3_WHITESPACES + PatternConstants.BULLETS
-            + "\\s*" //
-            + "(.*)"),
+            + "\\s*(" //
+            + PatternConstants.TAG_PATTERN + ")\\s*"),
     TITLE("#+\\s*(.*)"),
     UNDERLINE("([=-]{3,})\\s*");
     // @formatter:on
@@ -74,21 +76,4 @@ public LinePattern getPattern()
     {
         return this.pattern;
     }
-
-    private static final class PatternConstants
-    {
-        public static final String ARTIFACT_TYPE = "[a-zA-Z]+";
-        public static final String BULLETS = "[+*-]";
-        private static final String UP_TO_3_WHITESPACES = "\\s{0,3}";
-        // [impl->dsn~md.requirement-references~1]
-        public static final String REFERENCE_AFTER_BULLET = UP_TO_3_WHITESPACES
-                + PatternConstants.BULLETS + "(?:.*\\W)?" //
-                + "(" + SpecificationItemId.ID_PATTERN + ")" //
-                + "(?:\\W.*)?";
-
-        private PatternConstants()
-        {
-            // not instantiable
-        }
-    }
 }

importer/markdown/src/test/java/org/itsallcode/openfasttrace/importer/markdown/TestMarkdownImporter.java

@@ -86,10 +86,10 @@ void testIdentifyTags(final String text)
         MarkdownAsserts.assertMatch(MdPattern.TAGS_INT, text);
     }
 
+    // Note that this test does not evaluate the validity of the tags themselves. Only if the tags-marker is present.
     @ParameterizedTest
     @CsvSource(
-    { "Tags:", "#Needs: abc", "' Needs: abc'", "Needs: änderung", "Tags: -leadingDash", "Tags: trailingDash-",
-            "Tags: tag with spaces", "Tags: Täg" })
+    { "Tags:", "#Needs: abc", "' Needs: abc'", "Needs: änderung", })
     void testIdentifyNonTags(final String text)
     {
         MarkdownAsserts.assertMismatch(MdPattern.TAGS_INT, text);