Fix false positive detection heuristics

[alexzurbonsen]

Context

The newly introduced conditions on rule relevance and copyrights for false positive detections, see f9863e61, contain a bug that makes them detect copyrights and rule relevance if matches do not even have a matched_text or the rule doesn't even have a relevance attribute.

Fixes #4005 (at least for the example I am looking at)

Summary

Correct the any and all conditions to correctly detect copyrights and relevance.

Test plan

I reran the scan of the file from #4005

scancode -l <your path to boost repo>/boost/boost/assign/ptr_list_of.hpp --json scancode.json

and get a scancode.json that has the same contents as on v32.2.1.

scancode_fix.json

Tasks

• Reviewed contribution guidelines
• PR is descriptively titled 📑 and links the original issue above 🔗
• Tests pass -- look for a green checkbox ✔️ a few minutes after opening your PR
  Run tests locally to check for errors.
• Commits are in uniquely-named feature branch and has no merge conflicts 📁
• Updated documentation pages (if applicable)
• Updated CHANGELOG.rst (if applicable)

[alexzurbonsen]

@AyanSinhaMahapatra This should fix the issue with the false positive license detection heuristics. #4005

It feels like there should be a test for this. If so, can you help me write one? It seemed a bit more involved and I wanted to get this out quickly.

[alexzurbonsen]

There is a failing test tests/licensedcode/test_plugin_license_detection.py::test_complicated_license_text_from_ffmpeg It seems that it is relying on the buggy logic. I would be glad for some help on this one @AyanSinhaMahapatra, since I am not sure yet, what the appropriate change is.

[alexzurbonsen]

Update: I scanned plugin_license/scan/ffmpeg-LICENSE.md with the scancode version from this branch and pasted the result into the expected file. The changes seem reasonable to me.

[alexzurbonsen]

@AyanSinhaMahapatra and @pombredanne Would be great to get your feedback on this one. The bug currently blocking us from updating. It would seem that it basically renders the is_false_positive function useless, since it always returns false, and thus torpedoes false positive detection.

[alexzurbonsen]

Friendly ping @AyanSinhaMahapatra. It has been a few months, would be nice to get your feedback :)

[AyanSinhaMahapatra]

@alexzurbonsen apologies for the quite large delay on this and other PR reviews, looking at this now 😅

[AyanSinhaMahapatra]

@alexzurbonsen Thanks++ for spotting and fixing this bug, even though we had the right approach, we were not detecting false positives correctly because of this bug.

For now a simple test with a stripped version of the file at https://github.com/steinwurf/boost/blob/ade3189e2c03fd975dbfa667a4f49e98a49d2fdf/boost/assign/ptr_list_of.hpp#L196 (where gpl one word matches are correctly detected as clues) should suffice as a test to protect against any further regressions. Your test expectation updates also correct one such case from

scancode-toolkit/tests/licensedcode/data/plugin_license/scan/ffmpeg-LICENSE.md

Lines 100 to 101 in ee32157

	are incompatible with the GPLv2 and v3. We do not know for certain if their
	licenses are compatible with the LGPL.

In future we probably need to add one test case per case in the implementation of the is_false_positive function, i.e. better unit tests, which the detection.py lacks 😅 I will open a follow up issue on this.

This fixes #4005 and #3270 too (thanks @alok1304 for pointing this out.)

[AyanSinhaMahapatra]

The header portion is not required, we remove this to reduce test expectation updates.

[alexzurbonsen]

Hey @AyanSinhaMahapatra, thanks for the prompt review. I removed the header.

Do you want me to add the regression test against the boost file you mention?

[AyanSinhaMahapatra]

@alexzurbonsen we have this tested allright, so no worries. Merging!

[AyanSinhaMahapatra]

Thanks++ for adding this.