Column in DifferenceDescription returned by LicenseCompareHelper.isTextMatchingTemplate is not correct

[sdheh]

LicenseCompareHelper.isTextMatchingTemplate has the following code

compareTemplateOutputHandler = new CompareTemplateOutputHandler(removeLineSeparators(removeCommentChars(compareText)));

The column returned if there is a difference in a line from which comment chars were removed is wrong because it refers to the transformed text.

Example:

String licenseText = "// a b";
String template = "a";
System.out.println(LicenseCompareHelper.isTextMatchingTemplate(template, licenseText).getDifferenceMessage());

returns Additional text found after the end of the expected license text starting at line #1 column #3 "b"

[goneall]

Thanks again @sdheh for the analysis.

Pull requests are welcome - otherwise I'll look into a fix after finishing the SPDX 3.0 upgrade work.

[goneall]

@douglasclarke - do you think this is fixed with #310 ?

[douglasclarke]

Yes I believe this situation is better. The following test passes:

    @Test
    public void testAdditionalTextAfterWithComments() throws IOException, LicenseTemplateRuleException, LicenseParserException {
        String templateText = "a";
        String compareText = "// a b";
        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 #1 column #5 \"b\"", templateOutputHandler.getDifferences().getDifferenceMessage());
        assertEquals(1, templateOutputHandler.getDifferences().getDifferences().size());
        LineColumn lineColumn = templateOutputHandler.getDifferences().getDifferences().get(0);
        assertEquals(1, lineColumn.getLine());
        assertEquals(5, lineColumn.getColumn());
        assertEquals(1, lineColumn.getLen());

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

Note that lineColumn has zero based column index 

[goneall]

Thanks @douglasclarke - I'll go ahead and close this issue as resolved.

[douglasclarke]

One comment I would make is that the fix in #310 is based on the current implementation of LineColumn and src/test/java/org/spdx/licenseTemplate/LicenseTextHelperTest.java of spdx-java-core. I did test with trying to fix LineColumn to be 1 based for both line and column but the fix had many more transitive effects in license compare. Just noting that the fix provided is contextual on the underlying core's current functionality.

[goneall]

One comment I would make is that the fix in #310 is based on the current implementation of LineColumn and src/test/java/org/spdx/licenseTemplate/LicenseTextHelperTest.java of spdx-java-core. I did test with trying to fix LineColumn to be 1 based for both line and column but the fix had many more transitive effects in license compare. Just noting that the fix provided is contextual on the underlying core's current functionality.

Perhaps on the next release we can clean up the implementation of LineColumn - the code in license compare is rather complex, so I would expect there to be some additional changes.