add test for correct position of extra-words and enhance detection_log

Add test for is correct position of `extra-words` according to `extra-phrases` that is present in rules.

if we find `extra-words` are in the right place then we set score to `100`.
And also show in `detection_log` why we increasing the score to keep track of this.

Signed-off-by: Alok Kumar <[email protected]>

Author: alok1304

src/licensedcode/data/rules/bsd-new_158.RULE

@@ -14,7 +14,7 @@ Redistributions in binary form must reproduce the above copyright
 notice, this list of conditions and the following disclaimer in the
 documentation and/or other materials provided with the distribution.
 
-Neither the name of [[3]] nor the names of its
+Neither the name of [[6]] nor the names of its
 contributors may be used to endorse or promote products derived from
 this software without specific prior written permission.
 

src/licensedcode/data/rules/bsd-new_578.RULE

@@ -6,7 +6,9 @@ minimum_coverage: 99
 
 Software License Agreement (BSD License)
 
-Redistribution and use in source and binary forms, with or without
+[[15]]
+
+Redistribution and use [[4]] in source and binary forms, with or without
 modification, are permitted provided that the following conditions
 are met:
 
@@ -16,7 +18,7 @@ are met:
    copyright notice, this list of conditions and the following
    disclaimer in the documentation and/or other materials provided
    with the distribution.
- * Neither the name of  nor the names of its
+ * Neither the name of [[6]] nor the names of its
    contributors may be used to endorse or promote products derived
    from this software without specific prior written permission.
 

src/licensedcode/detection.py

@@ -30,6 +30,7 @@
 from licensedcode.cache import get_licensing
 from licensedcode.match import LicenseMatch
 from licensedcode.match import set_matched_lines
+from licensedcode.match import is_extra_words_position_valid
 from licensedcode.models import compute_relevance
 from licensedcode.models import Rule
 from licensedcode.models import UnDetectedRule
@@ -110,6 +111,7 @@ class DetectionCategory(Enum):
     PACKAGE_ADD_FROM_SIBLING_FILE = 'from-package-sibling-file'
     PACKAGE_ADD_FROM_FILE = 'from-package-file'
     EXTRA_WORDS = 'extra-words'
+    EXTRA_WORDS_PERMITTED = 'extra-words-permitted-in-rule'
     UNKNOWN_MATCH = 'unknown-match'
     LICENSE_CLUES = 'license-clues'
     LOW_QUALITY_MATCH_FRAGMENTS = 'low-quality-matches'
@@ -129,6 +131,7 @@ class DetectionRule(Enum):
     """
     UNKNOWN_MATCH = 'unknown-match'
     EXTRA_WORDS = 'extra-words'
+    EXTRA_WORDS_PERMITTED = 'extra-words-permitted-in-rule'
     LICENSE_CLUES = 'license-clues'
     LOW_QUALITY_MATCH_FRAGMENTS = 'low-quality-matches'
     IMPERFECT_COVERAGE = 'imperfect-match-coverage'
@@ -1063,6 +1066,7 @@ def is_correct_detection_non_unknown(license_matches):
         is_correct_detection(license_matches)
         and not has_unknown_matches(license_matches)
         and not has_extra_words(license_matches)
+        and not is_extra_words_at_valid_positions(license_matches)
     )  
 
 
@@ -1150,6 +1154,16 @@ def has_low_rule_relevance(license_matches):
     )
 
 
+def is_extra_words_at_valid_positions(license_matches):
+    """
+    Return True if any of the matches in ``license_matches`` List of LicenseMatch
+    has extra words are in the correct place.
+    """
+    return any(
+        is_extra_words_position_valid(license_match)
+        for license_match in license_matches
+    )
+
 def is_false_positive(license_matches, package_license=False):
     """
     Return True if all of the matches in ``license_matches`` List of LicenseMatch
@@ -1561,6 +1575,12 @@ def get_detected_license_expression(
         detection_log.append(DetectionRule.LOW_QUALITY_MATCH_FRAGMENTS.value)
         return detection_log, combined_expression
 
+    elif analysis == DetectionCategory.EXTRA_WORDS_PERMITTED.value:
+        if TRACE_ANALYSIS:
+            logger_debug(f'analysis {DetectionCategory.EXTRA_WORDS_PERMITTED.value}')
+        matches_for_expression = license_matches
+        detection_log.append(DetectionRule.EXTRA_WORDS_PERMITTED.value)
+    
     elif analysis == DetectionCategory.EXTRA_WORDS.value:
         if TRACE_ANALYSIS:
             logger_debug(f'analysis {DetectionCategory.EXTRA_WORDS.value}')
@@ -1798,6 +1818,10 @@ def analyze_detection(license_matches, package_license=False):
         threshold=IMPERFECT_MATCH_COVERAGE_THR,
     ):
         return DetectionCategory.IMPERFECT_COVERAGE.value
+    
+    # Case where `extra-words` are in the right place
+    elif is_extra_words_at_valid_positions(license_matches=license_matches):
+        return DetectionCategory.EXTRA_WORDS_PERMITTED.value
 
     # Case where at least one of the match have extra words
     elif has_extra_words(license_matches=license_matches):

src/licensedcode/index.py

@@ -391,9 +391,6 @@ def _add_rules(
             # "weak" rules can only be matched with an automaton exactly.
             is_weak = True
 
-            # identify and capture the spans of extra phrases specified within the rule
-            rule.extra_phrase_spans = list(rule.extra_phrases())
-         
             for rts in rule.tokens():
                 rule_tokens_append(rts)
                 rtid = dictionary_get(rts)

src/licensedcode/match.py

@@ -601,7 +601,7 @@ def score(self):
 
         # Check whether extra words in the matched text appear in allowed positions,
         # and do not exceed the maximum allowed word count at those positions.
-        if is_extra_words_position_valid(self):
+        if is_extra_words_position_valid(match=self):
             return 100
 
         # relevance is a number between 0 and 100. Divide by 100
@@ -1104,26 +1104,30 @@ def is_extra_words_position_valid(match):
     extra_phrase_count = 0
 
     for span, allowed_extra_words in extra_phrase_spans:
-        rule_index = span.start
+        rule_index = span.start - extra_phrase_count - 1
         allowed_extra_words = allowed_extra_words
 
-        matched_index = rule_index + matched_count - extra_phrase_count
+        matched_index = span.start + matched_count - extra_phrase_count
         extra_words_count = 0
 
-        # Count how many tokens in matched_text do not match the next rule token
+        # return false if token before `extra-words` in `matched_token` is not same as token before `extra-phrases` in `rule_tokens`
+        if(matched_tokens[matched_index-1] != rule_tokens[rule_index]):
+            return False 
+
+        # Count how many tokens in `matched_text` do not match the next rule token
         while (matched_index < len(matched_tokens) and
                matched_tokens[matched_index] != rule_tokens[rule_index + 1]):
             matched_index += 1
             matched_count += 1
             extra_words_count += 1
 
-        extra_phrase_count += 1
+            if extra_words_count > allowed_extra_words:
+               return False
 
-        if extra_words_count > allowed_extra_words:
-            return False
+        extra_phrase_count += 1
 
     return True
-        
+
 
 def filter_contained_matches(
     matches,

src/licensedcode/models.py

@@ -2319,6 +2319,9 @@ def load_data(self, rule_file):
         except Exception:
             trace = traceback.format_exc()
             raise InvalidRule(f'While loading: file://{rule_file}\n{trace}')
+        
+        # remove extra_phrase marker from rules
+        self.text = remove_extra_phrase(self.text)
 
         return self
 
@@ -2331,8 +2334,13 @@ def tokens(self):
         recomputed as a side effect.
         """
 
+        # identify and capture the spans of extra phrases specified within the rule
+        self.extra_phrase_spans = list(self.extra_phrases())
+        
         # remove extra_phrase marker from rules
-        text = remove_extra_phrase(self.text)
+        self.text = remove_extra_phrase(self.text)
+
+        text = self.text
 
         # We tag this rule as being a bare URL if it starts with a scheme and is
         # on one line: this is used to determine a matching approach

tests/licensedcode/data/plugin_license/extra-words/scan-extra-words-2-aho-license.expected.json

@@ -5,7 +5,9 @@
       "license_expression": "bsd-new",
       "license_expression_spdx": "BSD-3-Clause",
       "detection_count": 1,
-      "detection_log": [],
+      "detection_log": [
+        "extra-words-permitted-in-rule"
+      ],
       "reference_matches": [
         {
           "license_expression": "bsd-new",
@@ -54,7 +56,9 @@
               "matched_text_diagnostics": "Redistribution and use in source and binary forms, with or without\r\nmodification, are permitted provided that the following conditions are met:\r\n\r\n* Redistributions of source code must retain the above copyright notice, this\r\n  list of conditions and the following disclaimer.\r\n\r\n* Redistributions in binary form must reproduce the above copyright notice,\r\n  this list of conditions and the following disclaimer in the documentation\r\n  and/or other materials provided with the distribution.\r\n\r\n* Neither the name of [filesize] nor the names of its\r\n  contributors may be used to endorse or promote products derived from\r\n  this software without specific prior written permission.\r\n\r\nTHIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS \"AS IS\"\r\nAND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE\r\nIMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE\r\nDISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE\r\nFOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL\r\nDAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR\r\nSERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER\r\nCAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,\r\nOR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE\r\nOF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE."
             }
           ],
-          "detection_log": [],
+          "detection_log": [
+            "extra-words-permitted-in-rule"
+          ],
           "identifier": "bsd_new-f3efb258-e9c3-604d-9bbd-147fe74af22e"
         }
       ],

tests/licensedcode/data/plugin_license/extra-words/scan-extra-words-3-seq-license.expected.json

@@ -1,12 +1,12 @@
 {
   "license_detections": [
     {
-      "identifier": "bsd_new-95249a8d-f533-e7c7-159a-9b6e173cba42",
+      "identifier": "bsd_new-f327c6f9-6086-8bd5-22c6-3aee3b99acf2",
       "license_expression": "bsd-new",
       "license_expression_spdx": "BSD-3-Clause",
       "detection_count": 1,
       "detection_log": [
-        "extra-words"
+        "extra-words-permitted-in-rule"
       ],
       "reference_matches": [
         {
@@ -16,7 +16,7 @@
           "start_line": 1,
           "end_line": 31,
           "matcher": "3-seq",
-          "score": 93.89,
+          "score": 100,
           "matched_length": 215,
           "match_coverage": 100.0,
           "rule_relevance": 100,
@@ -46,7 +46,7 @@
               "start_line": 1,
               "end_line": 31,
               "matcher": "3-seq",
-              "score": 93.89,
+              "score": 100,
               "matched_length": 215,
               "match_coverage": 100.0,
               "rule_relevance": 100,
@@ -57,9 +57,9 @@
             }
           ],
           "detection_log": [
-            "extra-words"
+            "extra-words-permitted-in-rule"
           ],
-          "identifier": "bsd_new-95249a8d-f533-e7c7-159a-9b6e173cba42"
+          "identifier": "bsd_new-f327c6f9-6086-8bd5-22c6-3aee3b99acf2"
         }
       ],
       "license_clues": [],

tests/licensedcode/test_match.py

@@ -20,6 +20,7 @@
 from licensedcode.match import filter_overlapping_matches
 from licensedcode.match import get_full_matched_text
 from licensedcode.match import get_matching_regions
+from licensedcode.match import is_extra_words_position_valid
 from licensedcode.match import LicenseMatch
 from licensedcode.match import merge_matches
 from licensedcode.match import reportable_tokens
@@ -1321,6 +1322,106 @@ def test_get_matching_regions_3_lines_enough(self):
         assert matches[5].qspan in regions[1]
 
 
+class TestExtraWordsPosition(FileBasedTesting):
+    test_data_dir = TEST_DATA_DIR
+
+    def test_valid_extra_words_within_limit(self):
+        rule_text = """
+        Redistribution and use [[4]] in source and binary forms are permitted.
+        """
+        rule = create_rule_from_text_and_expression(
+            license_expression='extra-words',
+            text=rule_text
+        )
+        idx = index.LicenseIndex([rule])
+
+        query = """
+        Redistribution and use of this software in source and binary forms are permitted.
+        """
+        match = idx.match(query_string=query, _skip_hash_match=True)[0]
+        assert is_extra_words_position_valid(match) is True
+
+    def test_invalid_extra_words_exceed_limit(self):
+        rule_text = """
+        Redistribution and use [[2]] in source and binary forms are permitted.
+        """
+        rule = create_rule_from_text_and_expression(
+            license_expression='extra-words',
+            text=rule_text
+        )
+        idx = index.LicenseIndex([rule])
+
+        query = """
+        Redistribution and use of this software in source and binary forms are permitted.
+        """
+        match = idx.match(query_string=query, _skip_hash_match=True)[0]
+        assert is_extra_words_position_valid(match) is False
+
+    def test_no_extra_words_allowed(self):
+        rule_text = """
+        Redistribution and use in source and binary forms are permitted.
+        """
+        rule = create_rule_from_text_and_expression(
+            license_expression='extra-words',
+            text=rule_text
+        )
+        idx = index.LicenseIndex([rule])
+
+        query = """
+        Redistribution and use of software in source and binary forms are permitted.
+        """
+        match = idx.match(query_string=query, _skip_hash_match=True)[0]
+        assert is_extra_words_position_valid(match) is False
+
+    def test_multiple_extra_spans_valid(self):
+        rule_text = """
+        Redistribution [[2]] and use [[1]] in source and binary forms are permitted.
+        """
+        rule = create_rule_from_text_and_expression(
+            license_expression='extra-words',
+            text=rule_text
+        )
+        idx = index.LicenseIndex([rule])
+
+        query = """
+        Redistribution of content and use again in source and binary forms are permitted.
+        """
+        match = idx.match(query_string=query, _skip_hash_match=True)[0]
+        assert is_extra_words_position_valid(match) is True
+
+    def test_extra_words_at_wrong_position(self):
+        rule_text = """
+        Redistribution and use [[2]] in source and binary forms are permitted.
+        """
+        rule = create_rule_from_text_and_expression(
+            license_expression='extra-words',
+            text=rule_text
+        )
+        idx = index.LicenseIndex([rule])
+
+        query = """
+        Redistribution and amazing use in great source and binary forms are permitted.
+        """
+        match = idx.match(query_string=query, _skip_hash_match=True)[0]
+        assert is_extra_words_position_valid(match) is False
+
+    def test_exact_match_without_extra_markers(self):
+        rule_text = """
+        Redistribution and use in source and binary forms are permitted.
+        """
+        rule = create_rule_from_text_and_expression(
+            license_expression='extra-words',
+            text=rule_text
+        )
+        idx = index.LicenseIndex([rule])
+
+        query = """
+        Redistribution and use in source and binary forms are permitted.
+        """
+        match = idx.match(query_string=query, _skip_hash_match=True)[0]
+        assert is_extra_words_position_valid(match) is False
+
+
 class TestLicenseMatchScore(FileBasedTesting):
     test_data_dir = TEST_DATA_DIR
 

tests/licensedcode/test_tokenize.py

@@ -616,9 +616,9 @@ def test_get_extra_phrase_spans_simple(self):
         assert spans == [(Span([2]), 2)]
 
     def test_get_extra_phrase_spans_multiple(self):
-        text = 'Some [[1]] text [[3]] with multiple markers.'
+        text = 'Some [[4]] text [[6]] with multiple markers.'
         spans = get_extra_phrase_spans(text)
-        assert spans == [(Span([1]), 1), (Span([3]), 3)]
+        assert spans == [(Span([1]), 4), (Span([3]), 6)]
 
     def test_get_extra_phrase_spans_returns_nothing_if_none_found(self):
         text = 'Just some normal text.'