Skip to content

Issue 305: Address incorrect index offset for failed token in differences #310

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
goneall merged 3 commits into from
Apr 16, 2025
Merged

Issue 305: Address incorrect index offset for failed token in differences #310

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
045192b
Issue 305: Address incorrect index offset for failed token in differe…
douglasclarke Apr 15, 2025
5f8b9ae
fix update of org.spdx.utility.compare.TestCompareTemplateOutputHandl…
douglasclarke Apr 15, 2025
cf6c887
revert re-formatting changes
douglasclarke Apr 15, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/main/java/org/spdx/utility/compare/CompareTemplateOutputHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ public int match(String[] matchTokens, int startToken, int endToken, String orig
}
nextToken = compareText(textTokens, matchTokens, nextToken, this);
if (nextToken < 0) {
int errorLocation = -nextToken;
int errorLocation = -nextToken - 1;
differences.addDifference(tokenToLocation.get(errorLocation), LicenseTextHelper.getTokenAt(matchTokens, errorLocation),
"Normal text of license does not match", text, null, getLastOptionalDifference());
}
Expand Down
50 changes: 50 additions & 0 deletions src/test/java/org/spdx/utility/compare/TestCompareTemplateOutputHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import org.spdx.licenseTemplate.LicenseTemplateRuleException;
import org.spdx.licenseTemplate.SpdxLicenseTemplateHelper;
import org.spdx.licenseTemplate.LicenseTemplateRule.RuleType;
import org.spdx.licenseTemplate.LineColumn;

/**
* Test compare template output handler
Expand Down Expand Up @@ -625,4 +626,53 @@ public void testCompareHttps() throws IOException, LicenseTemplateRuleException,
fail(templateOutputHandler.getDifferences().getDifferenceMessage());
}
}

// Test for literal string matching without any var's
@Test
public void testCompareAlphabetDiffMessage() throws IOException, LicenseTemplateRuleException, LicenseParserException {
String templateText = "a b c d e f g h i j k l m n o p q r s t u v w x y z";
String compareText = "a b\tc d e f g h i j k l m n o p\nq r s t u v w x y zebra";
CompareTemplateOutputHandler templateOutputHandler = new CompareTemplateOutputHandler(compareText);
SpdxLicenseTemplateHelper.parseTemplate(templateText, templateOutputHandler);

System.out.println(templateOutputHandler.getDifferences().getDifferenceMessage());

if (templateOutputHandler.matches()) {
fail(templateOutputHandler.getDifferences().getDifferenceMessage());
}
assertEquals(String.format("Normal text of license does not match starting at line #2 column #18 \"zebra\" when comparing to template text \"%s\"", templateText), templateOutputHandler.getDifferences().getDifferenceMessage());
Copy link
Collaborator

@bact bact Apr 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure about the indexes inside LineColumn, but from this message and numbers 2 (line) and 18 (column) in two assertEquals statements, it looks like the index for Line starts at 1 and the index for Column starts at 0?

(we have one "\n" in the string = 2 lines; and "zebra" will be at column 18 only if "q" is at column 0)

If this is confirmed, may need to documented (in a separate PR) in https://github.com/spdx/spdx-java-core/blob/main/src/main/java/org/spdx/licenseTemplate/LineColumn.java since it is not intuitive.

Copy link
Contributor Author

@douglasclarke douglasclarke Apr 15, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agreed. My focus was on getting the correct token that failed in the match to help in comparing licenses with templates and diagnosing the failure.

Agree that line is 1 based and column is zero based in LineColumn. Assumed addressing that is a much bigger fix.

I believe this comes from SpdxLicenseTemplateHelper.class in spdx-java-core. We can work-around adding to the column value when creating the strings but is not ideal dealing with a side-effect that could change in the future.

Copy link
Member

@goneall goneall Apr 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll go ahead and merge this PR in for the release - we can create a separate issue / PR to address the line numbering issue.

assertEquals(1, templateOutputHandler.getDifferences().getDifferences().size());
LineColumn lineColumn = templateOutputHandler.getDifferences().getDifferences().get(0);
assertEquals(2, lineColumn.getLine());
assertEquals(18, lineColumn.getColumn());
assertEquals(5, lineColumn.getLen());

String line = compareText.split("\n")[lineColumn.getLine() - 1];
String failedToken = line.substring(lineColumn.getColumn(), lineColumn.getColumn() + lineColumn.getLen());
assertEquals("zebra", failedToken);
}

@Test
public void testCompareAlphabetVarDiffMessage() throws IOException, LicenseTemplateRuleException, LicenseParserException {
String templateText = "a<<var;name=\"more\";original=\"\";match=\"[b-y\\s]*\">><<beginOptional>>z<<endOptional>>";
String compareText = "a b\tc d e f g h i j k l m n o p\nq r s t u v w x y z end";
CompareTemplateOutputHandler templateOutputHandler = new CompareTemplateOutputHandler(compareText);
SpdxLicenseTemplateHelper.parseTemplate(templateText, templateOutputHandler);

System.out.println(templateOutputHandler.getDifferences().getDifferenceMessage());

if (templateOutputHandler.matches()) {
fail(templateOutputHandler.getDifferences().getDifferenceMessage());
}
assertEquals("Additional text found after the end of the expected license text starting at line #2 column #20 \"end\"", templateOutputHandler.getDifferences().getDifferenceMessage());
assertEquals(1, templateOutputHandler.getDifferences().getDifferences().size());
LineColumn lineColumn = templateOutputHandler.getDifferences().getDifferences().get(0);
assertEquals(2, lineColumn.getLine());
assertEquals(20, lineColumn.getColumn());
assertEquals(3, lineColumn.getLen());

String line = compareText.split("\n")[lineColumn.getLine() - 1];
String failedToken = line.substring(lineColumn.getColumn(), lineColumn.getColumn() + lineColumn.getLen());
assertEquals("end", failedToken);
}
}