diff --git a/api/src/main/java/org/itsallcode/openfasttrace/api/core/SpecificationItemId.java b/api/src/main/java/org/itsallcode/openfasttrace/api/core/SpecificationItemId.java index 1a83a889..6316798b 100644 --- a/api/src/main/java/org/itsallcode/openfasttrace/api/core/SpecificationItemId.java +++ b/api/src/main/java/org/itsallcode/openfasttrace/api/core/SpecificationItemId.java @@ -28,16 +28,22 @@ public final class SpecificationItemId implements Comparabledsn~md.specification-item-id-format~3] + // [impl->dsn~md.specification-item-id-format~4] private static final String ID = "(\\p{Alpha}+)" // + ARTIFACT_TYPE_SEPARATOR // + ITEM_NAME_PATTERN // - + REVISION_SEPARATOR // + + REVISION_SEPARATOR_PATTERN // + ITEM_REVISION_PATTERN; private static final String LEGACY_ID = LEGACY_ID_NAME + ", *v" // + ITEM_REVISION_PATTERN; @@ -137,6 +143,37 @@ public static SpecificationItemId parseId(final String idText) return new Builder(idText).build(); } + /** + * Create a specification item ID from an already-matched {@link #ID_PATTERN} + * matcher. + *

+ * The matcher must use {@link #ID_PATTERN} and must already have a current + * match from {@link Matcher#matches()} or {@link Matcher#find()}. This does + * not accept {@link #LEGACY_ID_PATTERN}. + *

+ * + * @param matcher + * an already-matched matcher whose pattern is {@link #ID_PATTERN} + * @return the specification item ID + * @throws IllegalArgumentException + * if the matcher's pattern is not {@link #ID_PATTERN}, or if the + * revision cannot be parsed + * @throws IllegalStateException + * if the matcher has no current match + */ + public static SpecificationItemId fromIdPatternMatcher(final Matcher matcher) + { + if (matcher.pattern() != ID_PATTERN) + { + throw new IllegalArgumentException( + "Matcher pattern must be SpecificationItemId.ID_PATTERN, but was '" + + matcher.pattern() + "'"); + } + final Builder builder = new Builder(); + builder.setIdPartsFromMatches(matcher); + return builder.build(); + } + /** * Create a specification item ID * @@ -174,7 +211,7 @@ public static SpecificationItemId createId(final String artifactType, final Stri */ public static class Builder { - // [impl->dsn~md.specification-item-id-format~3] + // [impl->dsn~md.specification-item-id-format~4] private final String id; private String artifactType; private String name; @@ -331,7 +368,7 @@ private void parseId() else { throw new IllegalArgumentException("Invalid specification item ID format: \"" + this.id - + "\". Format must be ~~."); + + "\". Format must be ~(~|#)."); } } } @@ -340,10 +377,10 @@ private void setIdPartsFromMatches(final Matcher matcher) { this.artifactType = matcher.group(ARTIFACT_TYPE_MATCHING_GROUP); this.name = matcher.group(NAME_MATCHING_GROUP); - parseRevision(matcher.group(REVISION_MATCHING_GROUP)); + parseRevision(matcher.group(REVISION_MATCHING_GROUP), matcher.group()); } - private void parseRevision(final String text) + private void parseRevision(final String text, final String idText) { try { @@ -352,7 +389,7 @@ private void parseRevision(final String text) catch (final NumberFormatException exception) { throw new IllegalArgumentException( - "Error parsing version number from specification item ID: \"" + this.id + "Error parsing version number from specification item ID: \"" + idText + "\"", exception); } diff --git a/api/src/test/java/org/itsallcode/openfasttrace/api/core/TestSpecificationItemId.java b/api/src/test/java/org/itsallcode/openfasttrace/api/core/TestSpecificationItemId.java index a1d3d62b..a6748b5a 100644 --- a/api/src/test/java/org/itsallcode/openfasttrace/api/core/TestSpecificationItemId.java +++ b/api/src/test/java/org/itsallcode/openfasttrace/api/core/TestSpecificationItemId.java @@ -4,8 +4,10 @@ import static org.hamcrest.Matchers.equalTo; import static org.itsallcode.openfasttrace.api.core.SpecificationItemId.createId; import static org.itsallcode.openfasttrace.api.core.SpecificationItemId.parseId; -import static org.junit.jupiter.api.Assertions.assertAll; -import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.*; + +import java.util.regex.Matcher; +import java.util.regex.Pattern; import org.itsallcode.openfasttrace.api.core.SpecificationItemId.Builder; import org.junit.jupiter.api.Test; @@ -15,7 +17,7 @@ import nl.jqno.equalsverifier.EqualsVerifier; /** - * [utest->dsn~md.specification-item-id-format~3] + * [utest->dsn~md.specification-item-id-format~4] */ // [utest->dsn~specification-item-id~1] class TestSpecificationItemId @@ -24,6 +26,7 @@ class TestSpecificationItemId @CsvSource(value = { "type~name~42, type, name, 42", + "type~name#42, type, name, 42", "type~name-with-dash~42, type, name-with-dash, 42", "type~name.with.dot~42, type, name.with.dot, 42", "type~name-trailing-~42, type, name-trailing-, 42", @@ -91,7 +94,7 @@ void testParseId_mustFailForIllegalIds(final String illegalId) final Throwable exception = assertThrows(IllegalArgumentException.class, () -> parseId(illegalId)); assertThat(exception.getMessage(), equalTo("Invalid specification item ID format: \"" + illegalId + "\". " - + "Format must be ~~.")); + + "Format must be ~(~|#).")); } @Test @@ -110,6 +113,14 @@ void testToString() assertThat(builder.build().toString(), equalTo("dsn~dummy~3")); } + @Test + void testHashRevisionSeparatorCreatesCanonicalId() + { + final SpecificationItemId id = parseId("dsn~dummy#3"); + assertAll(() -> assertThat(id, equalTo(parseId("dsn~dummy~3"))), + () -> assertThat(id.toString(), equalTo("dsn~dummy~3"))); + } + @Test void testToStringRevisionWildcard() { @@ -123,4 +134,49 @@ void testEqualsAndHashContract() { EqualsVerifier.forClass(SpecificationItemId.class).verify(); } + + @Test + void testFromIdPatternMatcherAfterMatches() + { + final Matcher matcher = SpecificationItemId.ID_PATTERN.matcher("dsn~dummy~3"); + assertTrue(matcher.matches()); + final SpecificationItemId id = SpecificationItemId.fromIdPatternMatcher(matcher); + assertAll( + () -> assertThat(id.getArtifactType(), equalTo("dsn")), + () -> assertThat(id.getName(), equalTo("dummy")), + () -> assertThat(id.getRevision(), equalTo(3))); + } + + @Test + void testFromIdPatternMatcherAfterFind() + { + final Matcher matcher = SpecificationItemId.ID_PATTERN.matcher("prefix dsn~dummy~3 suffix"); + assertTrue(matcher.find()); + final SpecificationItemId id = SpecificationItemId.fromIdPatternMatcher(matcher); + assertAll( + () -> assertThat(id.getArtifactType(), equalTo("dsn")), + () -> assertThat(id.getName(), equalTo("dummy")), + () -> assertThat(id.getRevision(), equalTo(3))); + } + + @Test + void testFromIdPatternMatcherRejectsWrongPattern() + { + final Matcher matcher = Pattern.compile("(\\p{Alpha}+)~(\\p{Alpha}+)~(\\d+)").matcher("dsn~dummy~3"); + assertTrue(matcher.matches()); + final IllegalArgumentException exception = assertThrows(IllegalArgumentException.class, + () -> SpecificationItemId.fromIdPatternMatcher(matcher)); + assertThat(exception.getMessage(), + equalTo("Matcher pattern must be SpecificationItemId.ID_PATTERN, but was '" + + matcher.pattern() + "'")); + } + + @Test + void testFromIdPatternMatcherUnmatchedThrowsIllegalStateException() + { + final Matcher matcher = SpecificationItemId.ID_PATTERN.matcher("dsn~dummy~3"); + final IllegalStateException exception = assertThrows(IllegalStateException.class, + () -> SpecificationItemId.fromIdPatternMatcher(matcher)); + assertThat(exception.getMessage(), equalTo("No match found")); + } } diff --git a/doc/changes/changes.md b/doc/changes/changes.md index 685c7150..45c95bc8 100644 --- a/doc/changes/changes.md +++ b/doc/changes/changes.md @@ -6,7 +6,8 @@ nav_order: 9 # Changes -* [4.11.0](changes_4.11.0.md) unreleased +* [4.11.0](changes_4.12.0.md) unreleased +* [4.11.0](changes_4.11.0.md) * [4.10.0](changes_4.10.0.md) * [4.9.0](changes_4.9.0.md) * [4.8.0](changes_4.8.0.md) diff --git a/doc/changes/changes_4.11.0.md b/doc/changes/changes_4.11.0.md index 2e552e8c..2396c032 100644 --- a/doc/changes/changes_4.11.0.md +++ b/doc/changes/changes_4.11.0.md @@ -22,6 +22,7 @@ The user guide documentation now has automatic lateral navigation. * #401: Added regression test for RST parser corner case (issue was fixed already) * #423: Fixed tag parsing inconsistency in Markdown and reStructuredText importers * #574: Fixed invalid HTML when inline code contains underscores +* #630: Accept `#` as an alternative separator between specification item names and revisions ## Documentation diff --git a/doc/changes/changes_4.12.0.md b/doc/changes/changes_4.12.0.md new file mode 100644 index 00000000..816c5bf0 --- /dev/null +++ b/doc/changes/changes_4.12.0.md @@ -0,0 +1,11 @@ +# OpenFastTrace 4.12.0, released 2026-10-?? + +Code name: Syntax usability improvements + +## Summary + +This release includes several usability improvements to the requirement ID and other syntax used in OpenFastTrace. For example, the hash mark (#) can now be used in addition to the tilde (~) as a revision separator. This prevents situations where the name part of the ID is displayed as cross-out text in Markdown. Of course, the previous double-tilde syntax remains valid. + +## Features + +* #638: Allow hash mark in addition to tilde as revision separator \ No newline at end of file diff --git a/doc/spec/design.md b/doc/spec/design.md index 3a3c819a..bc7ce44d 100644 --- a/doc/spec/design.md +++ b/doc/spec/design.md @@ -19,12 +19,10 @@ The terminology from the [central terminology document][bib.terminology] applies ## Conventions ### Syntax Definitions -Syntax definitions in this document use the [Augmented Backus-Naur Form][bib.abnf]. -The following definitions are used frequently throughout the document: +Syntax definitions in this document use the [Augmented Backus-Naur Form][bib.abnf]. -* ANY - any valid character -* LINEBREAK = the line break character of the platform +See [System Requirements, section "Syntax Definitions"](system-requirements#syntax-definitions) for more details. # Constraints @@ -957,37 +955,40 @@ Needs: impl, utest ### Markdown-style Structures #### Markdown Specification Item ID Format -`dsn~md.specification-item-id-format~3` +`dsn~md.specification-item-id-format~4` -A requirement ID has the following format +A specification item ID has the following format: - requirement-id = type "~" id "~" revision + requirement-id = artifact-type "~" name revision-separator revision - type = 1*ALPHA + artifact-type = 1*ALPHA - id = id-fragment *("." id-fragment) + name = unicode-alpha *(unicode-word / "-") + *("." 1*(unicode-word / "-")) - id-fragment = UNICODE_ALPHA *(UNICODE_ALPHA / DIGIT / "_" / "-") + revision-separator = "~" / "#" revision = 1*DIGIT Rationale: -* The ID may contain unicode letters to allow naming requirements using non-ASCII characters. This makes linking in formats like Markdown or HTML clean and easy. +* The ID may contain Unicode letters to allow naming requirements using non-ASCII characters. This makes linking in formats like Markdown or HTML clean and easy. * Requirement type and revision must be immediately recognizable from the requirement ID. -* The built-in revision number makes links break if a requirement is updated - a desired behavior. +* The built-in revision number makes links break if a requirement is updated โ€“ a desired behavior. +* `#` avoids Markdown interpreting an unquoted ID's two tildes as strikethrough markup. Comment: -Note that the artifact type is integral part of the ID. That means that `dsn~my-requirement~1` is something completely different then `utest~my-requirement~1`. One of the benefits of making the artifact type mandatory part of the ID is that this allows for typical coverage chains like. +Note that the artifact type is an integral part of the ID. That means that `dsn~my-requirement~1` is something completely different then `utest~my-requirement~1`. One of the benefits of making the artifact type mandatory part of the ID is that this allows for typical coverage chains like. req~my-requirement~2 -> dsn~my-requirement~4 -> impl~my-requirement~4 -Otherwise users would be forced to invent different names for each link in the chain. +Otherwise, users would be forced to invent different names for each link in the chain. Covers: * `req~markdown-standard-syntax~1` +* `req~specification-item-id-syntax~1` Needs: impl, utest @@ -1176,7 +1177,7 @@ OFT imports coverage tags in the full tag format with a list of required/needed Rationale: -The Tag importer is the catch all solution for all file formats that don't have a dedicated importer. We want to allow specification items imported via Tag importer to be intermediate nodes in the specification tree, instead of limiting them to leaves in the specification tree. Because leaves can only cover other specification items, but not require coverage. +The Tag importer is the catch-all solution for all file formats that don't have a dedicated importer. We want to allow specification items imported via Tag importer to be intermediate nodes in the specification tree, instead of limiting them to leaves in the specification tree. Because leaves can only cover other specification items but not require coverage. Especially when used for design document files like UML models, requiring coverage is useful. @@ -1205,13 +1206,13 @@ Covers: Needs: impl, utest #### Full Coverage Tag Format Allows Specifying a Revision -`dsn~import.full-coverage-tag-with-revision~1` +`dsn~import.full-coverage-tag-with-revision~2` OFT imports full coverage tags with an optional revision: - full-tag-with-revision = "[" *WSP reference "~" "~" revision "->" requirement-id "]" + full-tag-with-revision = "[" *WSP reference "~" revision-separator revision "->" requirement-id "]" - full-tag-with-revision-with-needed-coverage = "[" *WSP reference "~" "~" revision *WSP "->" *WSP requirement-id + full-tag-with-revision-with-needed-coverage = "[" *WSP reference "~" revision-separator revision *WSP "->" *WSP requirement-id *WSP ">>" *WSP artifact-type *WSP *("," *WSP artifact-type) "]" Rationale: @@ -1221,20 +1222,21 @@ Specifying an explicit revision in coverage tags allows incrementing the revisio Covers: * `req~import.full-coverage-tag-format~2` +* `req~specification-item-id-syntax~1` Needs: impl, utest #### Full Coverage Tag Format Allows Specifying Name and Revision -`dsn~import.full-coverage-tag-with-name-and-revision~1` +`dsn~import.full-coverage-tag-with-name-and-revision~2` OFT imports full coverage tags with optional name and revision: full-tag-with-name-and-revision = - "[" *WSP covering-artifact-type "~" covering-name "~" covering-revision + "[" *WSP covering-artifact-type "~" covering-name revision-separator covering-revision "->" covered-requirement-id "]" full-tag-with-name-and-revision-with-needed-coverage = - "[" *WSP covering-artifact-type "~" xxx "~" revision *WSP + "[" *WSP covering-artifact-type "~" xxx revision-separator revision *WSP "->" *WSP requirement-id *WSP ">>" *WSP artifact-type *WSP *("," *WSP artifact-type) "]" @@ -1245,6 +1247,7 @@ Specifying an explicit name in coverage tags allows overriding the auto-generate Covers: * `req~import.full-coverage-tag-format~2` +* `req~specification-item-id-syntax~1` Needs: impl, utest @@ -1255,7 +1258,7 @@ OFT generates readable names without hash code ID for tags with needed coverage. Rationale: -When you need to cover these items it's important that the name is predictable and does not change e.g. when the file changes. +When you need to cover these items, it's important that the name is predictable and does not change e.g. when the file changes. Covers: @@ -1385,7 +1388,7 @@ Needs: impl, itest #### Default Newline Format `dsn~cli.default-newline-format~1` -If the user does not specify the newline format as parameter, the exporter uses the native newline format of the platform OFT is executed on. +If the user does not specify the newline format as a parameter, the exporter uses the native newline format of the platform OFT is executed on. Covers: @@ -1514,11 +1517,11 @@ Needs: impl, utest ### Why is This Architecture Relevant? -Authors of importers need to be able to rely on these cleanups being done centrally, so that they don't have to implement them themselves. +Authors of importers need to be able to rely on these cleanups being done centrally so that they don't have to implement them themselves. ### Alternatives Considered -Clean-up in every importer individually. That was the case up to and including OFT 3.7.1 which turned out to make the importer more complex than necessary. +Cleanup in every importer individually. That was the case up to and including OFT 3.7.1, which turned out to make the importer more complex than necessary. # Bibliography @@ -1535,6 +1538,6 @@ The following documents or are referenced in this specification. * [OpenFastTrace Terminology][bib.terminology] * [Augmented BNF for Syntax Specifications: ABNF][bib.abnf], D. Crocker, P. Overell, January 2008 -## Web Sites +## Websites * [arc42 - Ressources for software architects][bib.arc42], Dr. Gernot Starke, Dr. Peter Hruschka diff --git a/doc/spec/system_requirements.md b/doc/spec/system_requirements.md index 92e6371c..b7217cc8 100644 --- a/doc/spec/system_requirements.md +++ b/doc/spec/system_requirements.md @@ -32,6 +32,25 @@ OFT aims to help developers independently of their platform. I should not matter OpenFastTrace uses unified terminology for all documents. Please refer to the [central terminology document](../terminology.md) for definitions of terms like [specification item](../terminology.md#specification-item), [coverage](../terminology.md#coverage), and [artifact](../terminology.md#artifact). +### Syntax Notation + +Syntax definitions in this document use [Augmented Backus-Naur Form (ABNF)](https://www.rfc-editor.org/rfc/rfc5234.html). +The following Unicode character classes are used in syntax definitions: + + unicode-alpha = + + unicode-word = unicode-alpha / unicode-nonspacing-mark / + unicode-spacing-combining-mark / unicode-decimal-digit / + unicode-connector-punctuation + + unicode-nonspacing-mark = + + unicode-spacing-combining-mark = + + unicode-decimal-digit = + + unicode-connector-punctuation = + ## Features ### Requirement Tracing @@ -121,19 +140,11 @@ Needs: dsn ### Located Specification Item IDs `req~located-specification-item-ids~1` -For every imported declared, covered, and dependency specification item ID, -OFT shall retain the individual source occurrence. For source text, positions -shall be zero-based UTF-16 offsets with start-inclusive, end-exclusive ranges. -Where an ID component is generated or its source is unavailable, its component -range shall be absent. Equal item IDs at distinct source occurrences shall -remain distinct occurrences. Short coverage tags shall not generate source -ranges for their generated or covered IDs. +For every imported declared, covered, and dependency specification item ID, OFT retains the individual source occurrence. For source text, positions are zero-based UTF-16 offsets with start-inclusive, end-exclusive ranges. Where an ID component is generated or its source is unavailable, its component range is absent. Equal item IDs at distinct source occurrences remain distinct occurrences. Short coverage tags do not generate source ranges for their generated or covered IDs. Rationale: -The exact location of an item ID is required for IDE plugins in order to support -features like syntax highlighting, find occurrences, jump to definition and auto-complete. -Adding this feature to OFT helps avoid code duplications and improves reliability. +The exact location of an item ID is required for IDE plugins to support features like syntax highlighting, find occurrences, jump to definition and auto-complete. Adding this feature to OFT helps avoid code duplications and improves reliability. Needs: dsn @@ -183,7 +194,7 @@ Needs: req ### Tracing Reports -A tracing report is a representation of the results of the requirement tracing OFT performs. Depending on their use, reports can be designed to be human readable, machine readable or both. +A tracing report is a representation of the results of the requirement tracing OFT performs. Depending on their use, reports can be designed to be human-readable, machine-readable or both. #### Console Reports @@ -281,6 +292,30 @@ Covers: Needs: dsn +#### Specification Item ID Syntax +`req~specification-item-id-syntax~1` + +A specification item ID consists of an artifact type, an item name, and a numeric revision. + + specification-item-id = artifact-type "~" name ("~" / "#") revision + + artifact-type = 1*(ALPHA) + + name = unicode-alpha *(unicode-word / "-") + *("." 1*(unicode-word / "-")) + + revision = 1*(DIGIT) + +Rationale: + +The `#` spelling lets authors use IDs in Markdown without the two tildes being interpreted as strikethrough markup. + +Covers: + +* [feat~requirement-tracing~1](#requirement-tracing) + +Needs: dsn + #### Forwarding Needed Coverage `req~forwarding_needed_coverage~1` diff --git a/doc/user_guide/introduction/concepts_and_terms.md b/doc/user_guide/introduction/concepts_and_terms.md index e17500e9..32ee0dc2 100644 --- a/doc/user_guide/introduction/concepts_and_terms.md +++ b/doc/user_guide/introduction/concepts_and_terms.md @@ -42,7 +42,7 @@ How many types you introduce, how you name and stack them is up to you. When we The name part of the ID must be a character string consisting of Unicode letters and/or numbers separated by underscore (`_`), hyphen (`-`) or dot (`.`). Whitespaces are not allowed. -* Names must start with a unicode letter +* Names must start with a Unicode letter * Consecutive dots `.` are not allowed We recommend using a dot `.` to create a hierarchy of items: @@ -55,7 +55,10 @@ We recommend using a dot `.` to create a hierarchy of items: The revision number of a specification item is a positive integer number that can be started at zero but by convention usually is started at one. -The revision is intended to obsolete existing coverage links in case the content of a specification item semantically changed. Incrementing the revision voids all existing links to this item so that authors linking to the item know they have to check for changes and adapt the covering items. +Separate the name and revision with either a tilde (`~`) or hash mark (`#`). For example, `req~example~1` and `req~example#1` identify the same item. Use +the hash mark when writing an unquoted ID in Markdown to avoid the two tildes being rendered as strikethrough markup. + +The revision is intended to obsolete existing coverage links in case the content of a specification item changed semantically. Incrementing the revision voids all existing links to this item so that authors linking to the item know they have to check for changes and adapt the covering items. Examples: diff --git a/doc/user_guide/use_cases/writing_a_specification.md b/doc/user_guide/use_cases/writing_a_specification.md index da5a3614..784cd8e5 100644 --- a/doc/user_guide/use_cases/writing_a_specification.md +++ b/doc/user_guide/use_cases/writing_a_specification.md @@ -23,6 +23,12 @@ Let's start with a minimal requirement: Simple as this. This is already a valid and complete OFT requirement. Of course, you can enrich the requirement with other information, but at the heart of it every requirement is an ID and a description. +When writing an ID without backticks in Markdown, you can use `#` before the revision to avoid Markdown interpreting the two tildes as strikethrough markup: + + req~this-is-the-id#1 + +This is the same specification item ID as `req~this-is-the-id~1`. + It is mostly a matter of taste whether you prefer your specification items to have a title or not. The same requirement above with a title looks like this: ### The Requirement Title diff --git a/importer/lightweightmarkup/src/main/java/org/itsallcode/openfasttrace/importer/lightweightmarkup/AbstractLightWeightMarkupImporter.java b/importer/lightweightmarkup/src/main/java/org/itsallcode/openfasttrace/importer/lightweightmarkup/AbstractLightWeightMarkupImporter.java index f1825d91..fa69142c 100644 --- a/importer/lightweightmarkup/src/main/java/org/itsallcode/openfasttrace/importer/lightweightmarkup/AbstractLightWeightMarkupImporter.java +++ b/importer/lightweightmarkup/src/main/java/org/itsallcode/openfasttrace/importer/lightweightmarkup/AbstractLightWeightMarkupImporter.java @@ -1,6 +1,7 @@ package org.itsallcode.openfasttrace.importer.lightweightmarkup; import java.util.logging.Logger; +import java.util.regex.Matcher; import java.util.regex.Pattern; import org.itsallcode.openfasttrace.api.core.*; @@ -277,15 +278,18 @@ protected void addCoverage() private LocatedSpecificationItemId locatedId(final String idText, final SpecificationItemId id) { // [impl->dsn~located-specification-item-id-text-ranges~1] + final Matcher matcher = SpecificationItemId.ID_PATTERN.matcher(idText); + if (!matcher.matches()) + { + throw new IllegalArgumentException("Expected a current specification item ID: " + idText); + } final int column = this.currentContext.currentLine().indexOf(idText); final SourceRange range = new SourceRange(new SourcePosition(this.currentContext.lineNumber() - 1, column), new SourcePosition(this.currentContext.lineNumber() - 1, column + idText.length())); - final int artifactTypeEnd = idText.indexOf(SpecificationItemId.ARTIFACT_TYPE_SEPARATOR); - final int revisionStart = idText.lastIndexOf(SpecificationItemId.REVISION_SEPARATOR) + 1; return LocatedSpecificationItemId.builder().id(id).range(range) - .artifactTypeRange(range(column, column + artifactTypeEnd)) - .nameRange(range(column + artifactTypeEnd + 1, column + revisionStart - 1)) - .revisionRange(range(column + revisionStart, column + idText.length())).build(); + .artifactTypeRange(range(column + matcher.start(1), column + matcher.end(1))) + .nameRange(range(column + matcher.start(2), column + matcher.end(2))) + .revisionRange(range(column + matcher.start(3), column + matcher.end(3))).build(); } private SourceRange range(final int start, final int end) diff --git a/importer/markdown/src/test/java/org/itsallcode/openfasttrace/importer/markdown/TestMarkdownImporter.java b/importer/markdown/src/test/java/org/itsallcode/openfasttrace/importer/markdown/TestMarkdownImporter.java index 7793879e..2db201a3 100644 --- a/importer/markdown/src/test/java/org/itsallcode/openfasttrace/importer/markdown/TestMarkdownImporter.java +++ b/importer/markdown/src/test/java/org/itsallcode/openfasttrace/importer/markdown/TestMarkdownImporter.java @@ -21,10 +21,10 @@ @ExtendWith(MockitoExtension.class) class TestMarkdownImporter { - // [utest->dsn~md.specification-item-id-format~3] + // [utest->dsn~md.specification-item-id-format~4] @ParameterizedTest @CsvSource( - { "req~foo~1", "a~b~0", "req~test~1", + { "req~foo~1", "a~b~0", "req~test~1", "req~test#1", "req~test~999", "req~test.requirement~1", "req~test_underscore~1", "`req~test1~1`arbitrary text", // See https://github.com/itsallcode/openfasttrace/issues/366 @@ -35,7 +35,7 @@ void testIdentifyId(final String text) MarkdownAsserts.assertMatch(MdPattern.ID, text); } - // [utest->dsn~md.specification-item-id-format~3] + // [utest->dsn~md.specification-item-id-format~4] @ParameterizedTest @CsvSource( { "test~1", "req-test~1", "req~4test~1", "rรคq~test~1" }) diff --git a/importer/markdown/src/test/java/org/itsallcode/openfasttrace/importer/markdown/TestMarkdownMarkupImporter.java b/importer/markdown/src/test/java/org/itsallcode/openfasttrace/importer/markdown/TestMarkdownMarkupImporter.java index 5b3fcefa..2c65c9c7 100644 --- a/importer/markdown/src/test/java/org/itsallcode/openfasttrace/importer/markdown/TestMarkdownMarkupImporter.java +++ b/importer/markdown/src/test/java/org/itsallcode/openfasttrace/importer/markdown/TestMarkdownMarkupImporter.java @@ -78,14 +78,19 @@ void testImportsLocatedDeclarationCoverageAndDependencyIds() final SpecificationItem item = importText(""" req~subject~1 Covers: - * req~covered~2 + * req~covered#2 Depends: - * req~dependency~3 + * req~dependency#3 """).get(0); assertAll( () -> assertThat(item.getLocatedId().getRange(), is(range(0, 0, 13))), + () -> assertThat(item.getLocatedId().getArtifactTypeRange().orElseThrow(), is(range(0, 0, 3))), + () -> assertThat(item.getLocatedId().getNameRange().orElseThrow(), is(range(0, 4, 11))), + () -> assertThat(item.getLocatedId().getRevisionRange().orElseThrow(), is(range(0, 12, 13))), () -> assertThat(item.getLocatedCoveredIds().get(0).getRange(), is(range(2, 2, 15))), + () -> assertThat(item.getLocatedCoveredIds().get(0).getNameRange().orElseThrow(), + is(range(2, 6, 13))), () -> assertThat(item.getLocatedDependOnIds().get(0).getRange(), is(range(4, 2, 18)))); } diff --git a/importer/restructuredtext/src/test/java/org/itsallcode/openfasttrace/importer/restructuredtext/TestRestructuredTextImporter.java b/importer/restructuredtext/src/test/java/org/itsallcode/openfasttrace/importer/restructuredtext/TestRestructuredTextImporter.java index 29d6f9e3..62c80a04 100644 --- a/importer/restructuredtext/src/test/java/org/itsallcode/openfasttrace/importer/restructuredtext/TestRestructuredTextImporter.java +++ b/importer/restructuredtext/src/test/java/org/itsallcode/openfasttrace/importer/restructuredtext/TestRestructuredTextImporter.java @@ -30,6 +30,18 @@ class TestRestructuredTextImporter extends AbstractLightWeightMarkupImporterTest super(1); } + // [utest->dsn~md.specification-item-id-format~4] + @Test + void testImportsHashRevisionSeparator() + { + assertImport("hash-revision.rst", """ + Hash Revision + ============= + req~hash-revision#1 + """, contains(item().title("Hash Revision").id("req", "hash-revision", 1) + .location("hash-revision.rst", 3).build())); + } + @Override protected ImporterFactory getImporterFactory() { diff --git a/importer/tag-importer-common/src/main/java/org/itsallcode/openfasttrace/importer/tag/common/LongTagImportingLineConsumer.java b/importer/tag-importer-common/src/main/java/org/itsallcode/openfasttrace/importer/tag/common/LongTagImportingLineConsumer.java index 0aeb24bc..c23fe880 100644 --- a/importer/tag-importer-common/src/main/java/org/itsallcode/openfasttrace/importer/tag/common/LongTagImportingLineConsumer.java +++ b/importer/tag-importer-common/src/main/java/org/itsallcode/openfasttrace/importer/tag/common/LongTagImportingLineConsumer.java @@ -22,7 +22,7 @@ class LongTagImportingLineConsumer extends AbstractRegexLineConsumer private static final String REVISION_GROUP = "revision"; private static final String COVERED_IDS_GROUP = "coveredIds"; private static final String COVERING_ARTIFACT_TYPE_PATTERN = "\\p{Alpha}+"; - // [impl->dsn~import.full-coverage-tag-with-revision~1] + // [impl->dsn~import.full-coverage-tag-with-revision~2] private static final String OPTIONAL_WHITESPACE = "\\s*"; private static final String TAG_PREFIX = "\\["; private static final String TAG_SUFFIX = "\\]"; @@ -35,9 +35,9 @@ class LongTagImportingLineConsumer extends AbstractRegexLineConsumer private static final String TAG_REGEX = TAG_PREFIX + OPTIONAL_WHITESPACE + "(?<" + ARTIFACT_TYPE_GROUP + ">" + COVERING_ARTIFACT_TYPE_PATTERN + ")" + "(?:" + SpecificationItemId.ARTIFACT_TYPE_SEPARATOR - // [impl->dsn~import.full-coverage-tag-with-name-and-revision~1] + // [impl->dsn~import.full-coverage-tag-with-name-and-revision~2] + "(?<" + CUSTOM_NAME_GROUP + ">" + SpecificationItemId.ITEM_NAME_PATTERN + ")?" - + SpecificationItemId.REVISION_SEPARATOR + + SpecificationItemId.REVISION_SEPARATOR_PATTERN + "(?<" + REVISION_GROUP + ">" + SpecificationItemId.ITEM_REVISION_PATTERN + "))?" + OPTIONAL_WHITESPACE + "->" + OPTIONAL_WHITESPACE + "(?<" + COVERED_IDS_GROUP + ">" + COVERED_IDS + ")" @@ -57,49 +57,61 @@ class LongTagImportingLineConsumer extends AbstractRegexLineConsumer @Override public void processMatch(final Matcher matcher, final int lineNumber, final int lineMatchCount) { - final List coveredIds = parseCoveredIds(matcher.group(COVERED_IDS_GROUP)); + final List locatedCoveredIds = locateCoveredIds(lineNumber, matcher); + final List coveredIds = locatedCoveredIds.stream() + .map(LocatedSpecificationItemId::getId) + .toList(); final List neededArtifactTypes = parseNeededArtifactTypes(matcher.group("neededArtifactTypes")); final List generatedIds = createItemIds(matcher, lineNumber, lineMatchCount, coveredIds, neededArtifactTypes); if (generatedIds.size() > 1) { - assert generatedIds.size() == coveredIds.size(); + assert generatedIds.size() == locatedCoveredIds.size(); for (int i = 0; i < generatedIds.size(); i++) { - addSpecificationItem(lineNumber, matcher, generatedIds.get(i), List.of(coveredIds.get(i)), + addSpecificationItem(lineNumber, matcher, generatedIds.get(i), List.of(locatedCoveredIds.get(i)), neededArtifactTypes); } } else { - addSpecificationItem(lineNumber, matcher, generatedIds.get(0), coveredIds, neededArtifactTypes); + addSpecificationItem(lineNumber, matcher, generatedIds.get(0), locatedCoveredIds, neededArtifactTypes); } } private void addSpecificationItem(final int lineNumber, final Matcher matcher, - final SpecificationItemId generatedId, final List coveredIds, + final SpecificationItemId generatedId, final List coveredIds, final List neededArtifactTypes) { final SpecificationItem.Builder item = SpecificationItem.builder() .id(locatedGeneratedId(lineNumber, matcher, generatedId)) .location(this.file.getPath(), lineNumber); - int searchStart = 0; - for (final SpecificationItemId coveredId : coveredIds) - { - final int start = matcher.group(COVERED_IDS_GROUP).indexOf(coveredId.toString(), searchStart); - searchStart = start + coveredId.toString().length(); - item.addCoveredId(locatedId(lineNumber, matcher.start(COVERED_IDS_GROUP) + start, coveredId)); - } + coveredIds.forEach(item::addCoveredId); neededArtifactTypes.forEach(item::addNeedsArtifactType); this.listener.addSpecificationItem(item.build()); - logItem(lineNumber, coveredIds, neededArtifactTypes, generatedId); + logItem(lineNumber, coveredIds.stream().map(LocatedSpecificationItemId::getId).toList(), neededArtifactTypes, + generatedId); + } + + // [impl->dsn~located-specification-item-id-tag-ranges~1] + private static List locateCoveredIds(final int lineNumber, final Matcher matcher) + { + final String coveredIdsText = matcher.group(COVERED_IDS_GROUP); + final int coveredIdsStart = matcher.start(COVERED_IDS_GROUP); + final Matcher idMatcher = SpecificationItemId.ID_PATTERN.matcher(coveredIdsText); + final List located = new ArrayList<>(); + while (idMatcher.find()) + { + located.add(locatedId(lineNumber, coveredIdsStart, idMatcher, + SpecificationItemId.fromIdPatternMatcher(idMatcher))); + } + return located; } private static LocatedSpecificationItemId locatedGeneratedId(final int lineNumber, final Matcher matcher, final SpecificationItemId id) { - // [impl->dsn~located-specification-item-id-tag-ranges~1] if (matcher.group(CUSTOM_NAME_GROUP) == null) { return LocatedSpecificationItemId.builder().id(id).build(); @@ -113,16 +125,17 @@ private static LocatedSpecificationItemId locatedGeneratedId(final int lineNumbe .build(); } - private static LocatedSpecificationItemId locatedId(final int lineNumber, final int start, - final SpecificationItemId id) + private static LocatedSpecificationItemId locatedId(final int lineNumber, final int coveredIdsStart, + final Matcher matcher, final SpecificationItemId id) { - final String text = id.toString(); - final int typeEnd = text.indexOf('~'); - final int revisionStart = text.lastIndexOf('~') + 1; - return LocatedSpecificationItemId.builder().id(id).range(sourceRange(lineNumber, start, start + text.length())) - .artifactTypeRange(sourceRange(lineNumber, start, start + typeEnd)) - .nameRange(sourceRange(lineNumber, start + typeEnd + 1, start + revisionStart - 1)) - .revisionRange(sourceRange(lineNumber, start + revisionStart, start + text.length())).build(); + return LocatedSpecificationItemId.builder().id(id) + .range(sourceRange(lineNumber, coveredIdsStart + matcher.start(), coveredIdsStart + matcher.end())) + .artifactTypeRange(sourceRange(lineNumber, coveredIdsStart + matcher.start(1), + coveredIdsStart + matcher.end(1))) + .nameRange(sourceRange(lineNumber, coveredIdsStart + matcher.start(2), + coveredIdsStart + matcher.end(2))) + .revisionRange(sourceRange(lineNumber, coveredIdsStart + matcher.start(3), + coveredIdsStart + matcher.end(3))).build(); } private static SourceRange sourceRange(final int lineNumber, final int start, final int end) @@ -130,19 +143,6 @@ private static SourceRange sourceRange(final int lineNumber, final int start, fi return new SourceRange(new SourcePosition(lineNumber - 1, start), new SourcePosition(lineNumber - 1, end)); } - private static List parseCoveredIds(final String input) - { - if (input == null) - { - return emptyList(); - } - return Arrays.stream(input.split(",")) - .map(String::trim) - .filter(Predicate.not(String::isEmpty)) - .map(SpecificationItemId::parseId) - .toList(); - } - private static List parseNeededArtifactTypes(final String input) { if (input == null) diff --git a/importer/tag-importer-common/src/test/java/org/itsallcode/openfasttrace/importer/tag/common/TestLongTagImportingLineConsumer.java b/importer/tag-importer-common/src/test/java/org/itsallcode/openfasttrace/importer/tag/common/TestLongTagImportingLineConsumer.java index 43f25933..bbd7938b 100644 --- a/importer/tag-importer-common/src/test/java/org/itsallcode/openfasttrace/importer/tag/common/TestLongTagImportingLineConsumer.java +++ b/importer/tag-importer-common/src/test/java/org/itsallcode/openfasttrace/importer/tag/common/TestLongTagImportingLineConsumer.java @@ -29,7 +29,7 @@ static Stream tagImportingTests() final String generatedNameTag = "[impl -> dsn~covered~2" + "]"; final String readableNamesTag = "[impl -> dsn~first~2, dsn~second~3 >> utest" + "]"; return Stream.of( - // [utest->dsn~import.full-coverage-tag-with-name-and-revision~1] + // [utest->dsn~import.full-coverage-tag-with-name-and-revision~2] // [utest->dsn~import.full-coverage-tag-with-needed-coverage~1] // [utest->dsn~import.full-coverage-tag-multiple-needed-coverage~1] testCase(3, namedTag, @@ -75,6 +75,23 @@ void importsLocatedLongTagIds() () -> assertThat(item.getLocatedCoveredIds().get(0).getRange(), is(range(2, 18, 31)))); } + // [utest->dsn~md.specification-item-id-format~4] + // [utest->dsn~import.full-coverage-tag-with-name-and-revision~2] + // [utest->dsn~located-specification-item-id-tag-ranges~1] + @Test + void testImportsLocatedLongTagIdsWithHashRevisionSeparator() + { + final SpecificationListBuilder listener = SpecificationListBuilder.create(); + new LongTagImportingLineConsumer(inputFile(), listener).readLine(3, + "๐Ÿ˜€ [impl~tag#1 -> dsn~covered#2" + "]"); + + final SpecificationItem item = listener.build().get(0); + + assertAll( + () -> assertThat(item.getLocatedId().getRange(), is(range(2, 4, 14))), + () -> assertThat(item.getLocatedCoveredIds().get(0).getRange(), is(range(2, 18, 31)))); + } + // [utest->dsn~located-specification-item-id-tag-ranges~1] @Test void importsLocatedLongTagIdWithGeneratedName() @@ -90,6 +107,42 @@ void importsLocatedLongTagIdWithGeneratedName() () -> assertThat(item.getLocatedCoveredIds().get(0).getRange(), is(range(2, 12, 25)))); } + // [utest->dsn~located-specification-item-id-tag-ranges~1] + @Test + void importsSecondGeneratedItemCoveredIdRange() + { + final SpecificationListBuilder listener = SpecificationListBuilder.create(); + new LongTagImportingLineConsumer(inputFile(), listener).readLine(3, + "๐Ÿ˜€ [impl -> dsn~first~2, dsn~second~3 >> utest" + "]"); + + final List items = listener.build(); + assertAll( + () -> assertThat(items, hasSize(2)), + () -> assertThat(items.get(0).getLocatedId().getRange(), is(nullValue())), + () -> assertThat(items.get(1).getLocatedId().getRange(), is(nullValue())), + () -> assertThat(items.get(0).getLocatedCoveredIds(), hasSize(1)), + () -> assertThat(items.get(1).getLocatedCoveredIds(), hasSize(1)), + () -> assertThat(items.get(0).getLocatedCoveredIds().get(0).getRange(), is(range(2, 12, 23))), + () -> assertThat(items.get(1).getLocatedCoveredIds().get(0).getRange(), is(range(2, 25, 37)))); + } + + // [utest->dsn~located-specification-item-id-tag-ranges~1] + @Test + void importsDuplicateCoveredIdsWithDistinctRanges() + { + final SpecificationListBuilder listener = SpecificationListBuilder.create(); + new LongTagImportingLineConsumer(inputFile(), listener).readLine(3, + "๐Ÿ˜€ [impl~tag~1 -> dsn~same~1, dsn~same~1" + "]"); + + final SpecificationItem item = listener.build().get(0); + assertAll( + () -> assertThat(item.getLocatedCoveredIds(), hasSize(2)), + () -> assertThat(item.getLocatedCoveredIds().get(0).getId(), + equalTo(item.getLocatedCoveredIds().get(1).getId())), + () -> assertThat(item.getLocatedCoveredIds().get(0).getRange(), is(range(2, 18, 28))), + () -> assertThat(item.getLocatedCoveredIds().get(1).getRange(), is(range(2, 30, 40)))); + } + private static SourceRange range(final int line, final int start, final int end) { return new SourceRange(new SourcePosition(line, start), new SourcePosition(line, end)); diff --git a/importer/tag/src/test/java/org/itsallcode/openfasttrace/importer/tag/TestTagImporter.java b/importer/tag/src/test/java/org/itsallcode/openfasttrace/importer/tag/TestTagImporter.java index 3cff0181..8eca0dce 100644 --- a/importer/tag/src/test/java/org/itsallcode/openfasttrace/importer/tag/TestTagImporter.java +++ b/importer/tag/src/test/java/org/itsallcode/openfasttrace/importer/tag/TestTagImporter.java @@ -98,6 +98,11 @@ static Stream tagImporterTests() itemBuilder().id(SpecificationItemId.parseId("impl~combined~1")) .addCoveredId(SpecificationItemId.parseId("dsn~name1~2")) .addCoveredId(SpecificationItemId.parseId("dsn~name2~3"))), + // [utest->dsn~md.specification-item-id-format~4] + // [utest->dsn~import.full-coverage-tag-with-name-and-revision~2] + parsedItem("[impl~combined#1->dsn~name1#2" + "]", + itemBuilder().id(SpecificationItemId.parseId("impl~combined~1")) + .addCoveredId(SpecificationItemId.parseId("dsn~name1~2"))), parsedItem("[ impl~combined~1 -> dsn~name1~2 , dsn~name2~3 >> test" + "]", itemBuilder().id(SpecificationItemId.parseId("impl~combined~1")) .addCoveredId(SpecificationItemId.parseId("dsn~name1~2")) @@ -191,7 +196,7 @@ static Stream tagImporterTests() .addNeedsArtifactType("impl") .addNeedsArtifactType("test")), - // [utest->dsn~import.full-coverage-tag-with-revision~1] + // [utest->dsn~import.full-coverage-tag-with-revision~2] parsedItem("[impl~~42->req~name~17" + "]", itemACoveringB("impl~name-3433816440~42", "req~name~17")), parsedItems("[impl~~42->req~name~17" + "][impl~~42->req~name~17" + "]", @@ -199,12 +204,12 @@ static Stream tagImporterTests() itemACoveringB("impl~name-1460579607~42", "req~name~17")), noItemDetected("[impl~~-42->req~name~17]"), - // [utest->dsn~import.full-coverage-tag-with-revision~1] + // [utest->dsn~import.full-coverage-tag-with-revision~2] parsedItem("[impl~~42->req~example_name~17>>test" + "]", itemACoveringB("impl~example_name~42", "req~example_name~17") .addNeedsArtifactType("test")), - // [utest->dsn~import.full-coverage-tag-with-name-and-revision~1] + // [utest->dsn~import.full-coverage-tag-with-name-and-revision~2] parsedItem("[impl->dsn~name~2" + "]", itemACoveringB("impl~name-1627661772~0", "dsn~name~2")), parsedItem("[impl~name1~1->dsn~name2~2" + "]", diff --git a/testutil/src/main/java/org/itsallcode/openfasttrace/testutil/importer/lightweightmarkup/AbstractLightWeightMarkupImporterTest.java b/testutil/src/main/java/org/itsallcode/openfasttrace/testutil/importer/lightweightmarkup/AbstractLightWeightMarkupImporterTest.java index 2aa6da41..091e780c 100644 --- a/testutil/src/main/java/org/itsallcode/openfasttrace/testutil/importer/lightweightmarkup/AbstractLightWeightMarkupImporterTest.java +++ b/testutil/src/main/java/org/itsallcode/openfasttrace/testutil/importer/lightweightmarkup/AbstractLightWeightMarkupImporterTest.java @@ -52,9 +52,10 @@ protected AbstractLightWeightMarkupImporterTest(final int titleLocationOffset) */ protected abstract String formatTitle(final String title, int level); - // [utest -> dsn~md.specification-item-id-format~3] + // [utest -> dsn~md.specification-item-id-format~4] @CsvSource({ "at~name~67, at, name, 67", + "at~name#67, at, name, 67", "longartifacttype~name.with.dots~123456789, longartifacttype, name.with.dots, 123456789", "a~b1.c1.d~0, a, b1.c1.d, 0" }) @@ -578,7 +579,7 @@ void testItemStatsRecognized() .build())); } - // [impl -> dsn~md.specification-item-id-format~3] with UTF-8 + // [impl -> dsn~md.specification-item-id-format~4] with UTF-8 @Test void testItemIdSupportsUTF8Characaters() {