Issue 305: Address incorrect index offset for failed token in differences#310
Issue 305: Address incorrect index offset for failed token in differences#310goneall merged 3 commits intospdx:masterfrom
Conversation
…nces. Added test cases
…er t only have new test cases
| 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()); |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
3 participants
Current impl reports the failed index on the next token or null if already at last token.
This minor fix corrects the errorLocation based on the token matching incrementing index before failing
Added two test cases to validate with pure text abd var template matches.