Repository: ishepard/pydriller
Pull request #220 (2022-06-14 10:12:13)

Adding ModifiedFile.content and deprecating ModifiedFile.source_code
This pull request should address #216, open to discuss modifications and refactoring operations.

I believe `source_code` is a core property used by many and therefore should not be refactored to `content` any time soon. Instead, it should be deprecated and then removed in a future version of PyDriller. Therefore, this pull request addresses issue #216 by adding the class variables `content` and `content_before` that behave exactly like `source_code` and `source_code_before`; the latter have been also translated to properties to warn the user about their deprecation. 

Test cases using property `source_code` and `source_code_before`. Checked :ballot_box_with_check:  if modified for DeprecationWarning and testing the new property `content`:

- [x] **tests/test_commit/test_source_code_before()**
- [x] **tests/test_commit/test_source_code_before_complete()**
- [x] **tests/test_commit/test_filename()**
- [x] **tests/test_commit/test_metrics_python()**
- [x] **tests/test_commit/test_metrics_cpp()**
- [x] **tests/test_commit/test_metrics_java()**
- [x] **tests/test_commit/test_metrics_not_supported_file()**
- [x] **tests/test_git_repository/test_should_detail_a_commit()**

Methods in `ModifiedFile` that use `source_code` or `source_code_before`. Checked :ballot_box_with_check:  if modified to warn deprecation and to include the new property `content`:

- [x] __init__()
- `self.source_code` has been renamed to `self.__source_code` to handle DeprecationWarning in a new method/property `source_code()`.
- `self.source_code_before` has been renamed to `self.__source_code_before` to handle DeprecationWarning in a new method/property `source_code_before()`.
- Added `self.content` and `self.content_before`.
- [x] __hash__()
- `self.source_code` replaced by `self.content` to generate hashable string. 
- [ ] **_calculate_metrics()**
- Refactor `source_code` and `source_code_before` to `content` and `content_before`.


Methods in `Commit` that use `source_code` or `source_code_before`. 
- [x] **_parse_diff()**
- Added key:value pairs `content_before` and `content` to the `diff_and_sc` dictionary that is passed to ModifiedFile.

Future refactoring:
- [ ] Rename `diff_and_sc -> diff_and_content` in commit.py.
 



Patch:
diff --git a/pydriller/domain/commit.py b/pydriller/domain/commit.py
index 6981b6f..e598f61 100644
--- a/pydriller/domain/commit.py
+++ b/pydriller/domain/commit.py
@@ -22,6 +22,7 @@ from _datetime import datetime
 from enum import Enum
 from pathlib import Path
 from typing import List, Set, Dict, Tuple, Optional
+from warnings import warn
 
 import hashlib
 
@@ -142,7 +143,7 @@ class Method:
             return self.complexity <= Method.UNIT_COMPLEXITY_LOW_RISK_THRESHOLD
         assert dmm_prop is DMMProperty.UNIT_INTERFACING
         return (
-            len(self.parameters) <= Method.UNIT_INTERFACING_LOW_RISK_THRESHOLD
+                len(self.parameters) <= Method.UNIT_INTERFACING_LOW_RISK_THRESHOLD
         )
 
 
@@ -152,11 +153,11 @@ class ModifiedFile:
     """
 
     def __init__(
-        self,
-        old_path: Optional[str],
-        new_path: Optional[str],
-        change_type: ModificationType,
-        diff_and_sc: Dict[str, str],
+            self,
+            old_path: Optional[str],
+            new_path: Optional[str],
+            change_type: ModificationType,
+            diff_and_sc: Dict[str, str],
     ):
         """
         Initialize a modified file. A modified file carries on information
@@ -167,8 +168,10 @@ class ModifiedFile:
         self._new_path = Path(new_path) if new_path is not None else None
         self.change_type = change_type
         self.diff = diff_and_sc["diff"]
-        self.source_code = diff_and_sc["source_code"]
-        self.source_code_before = diff_and_sc["source_code_before"]
+        self.__source_code = diff_and_sc["source_code"]
+        self.__source_code_before = diff_and_sc["source_code_before"]
+        self.content = diff_and_sc["content"]
+        self.content_before = diff_and_sc["content_before"]
 
         self._nloc = None
         self._complexity = None
@@ -183,9 +186,27 @@ class ModifiedFile:
 
         :return: int hash
         """
-        string = f"{self.change_type.name} {self.new_path} {self.source_code}"
+        string = f"{self.change_type.name} {self.new_path} {self.content}"
         return hash(hashlib.sha256(string.encode("utf-8")).hexdigest())
 
+    @property
+    def source_code(self):
+        warn('The use of `source_code` is deprecated. Use `content` instead.', DeprecationWarning, stacklevel=2)
+        return self.__source_code
+
+    @source_code.setter
+    def source_code(self, value: str):
+        self.__source_code = value
+
+    @property
+    def source_code_before(self):
+        warn('The use of `source_code_before` is deprecated. Use `content_before` instead.', DeprecationWarning, stacklevel=2)
+        return self.__source_code_before
+
+    @source_code_before.setter
+    def source_code_before(self, value: str):
+        self.__source_code_before = value
+
     @property
     def added_lines(self) -> int:
         """
@@ -340,7 +361,7 @@ class ModifiedFile:
         numbers_old_file = token[1]
         numbers_new_file = token[2]
         delete_line_number = (
-            int(numbers_old_file.split(",")[0].replace("-", "")) - 1
+                int(numbers_old_file.split(",")[0].replace("-", "")) - 1
         )
         additions_line_number = int(numbers_new_file.split(",")[0]) - 1
         return delete_line_number, additions_line_number
@@ -400,7 +421,7 @@ class ModifiedFile:
 
     @staticmethod
     def _risk_profile(
-        methods: List[Method], dmm_prop: DMMProperty
+            methods: List[Method], dmm_prop: DMMProperty
     ) -> Tuple[int, int]:
         """
         Return the risk profile of the set of methods, with two bins: risky, or non risky.
@@ -452,9 +473,9 @@ class ModifiedFile:
                 self._function_list.append(Method(func))
 
         if (
-            include_before
-            and self.source_code_before
-            and not self._function_list_before
+                include_before
+                and self.source_code_before
+                and not self._function_list_before
         ):
             anal = lizard.analyze_file.analyze_source_code(
                 self.filename, self.source_code_before
@@ -714,6 +735,8 @@ class Commit:
                 "diff": self._get_decoded_str(diff.diff),
                 "source_code_before": self._get_decoded_sc_str(diff.a_blob),
                 "source_code": self._get_decoded_sc_str(diff.b_blob),
+                "content_before": self._get_decoded_sc_str(diff.a_blob),
+                "content": self._get_decoded_sc_str(diff.b_blob),
             }
 
             modified_files_list.append(
@@ -847,7 +870,7 @@ class Commit:
         return None
 
     def _delta_risk_profile(
-        self, dmm_prop: DMMProperty
+            self, dmm_prop: DMMProperty
     ) -> Optional[Tuple[int, int]]:
         """
         Return the delta risk profile of this commit, which a pair (dv1, dv2), where
@@ -872,7 +895,7 @@ class Commit:
 
     @staticmethod
     def _good_change_proportion(
-        low_risk_delta: int, high_risk_delta: int
+            low_risk_delta: int, high_risk_delta: int
     ) -> Optional[float]:
         """
         Given a delta risk profile, compute the proportion of "good" change in the total change.


Test patch:
diff --git a/tests/test_commit.py b/tests/test_commit.py
index 8be3efb..0e89dcf 100644
--- a/tests/test_commit.py
+++ b/tests/test_commit.py
@@ -13,6 +13,7 @@
 # limitations under the License.
 from pydriller.git import Git
 from pathlib import Path
+from warnings import catch_warnings
 import pytest
 import logging
 
@@ -44,7 +45,9 @@ def test_filename():
     diff_and_sc = {
         'diff': '',
         'source_code': '',
-        'source_code_before': ''
+        'source_code_before': '',
+        'content': '',
+        'content_before': ''
     }
     m1 = ModifiedFile('dspadini/pydriller/myfile.py',
                       'dspadini/pydriller/mynewfile.py',
@@ -69,7 +72,9 @@ def test_metrics_python():
     diff_and_sc = {
         'diff': '',
         'source_code': sc,
-        'source_code_before': sc
+        'source_code_before': sc,
+        'content': sc,
+        'content_before': sc
     }
 
     m1 = ModifiedFile('test-repos/lizard/git_repository.py',
@@ -143,7 +148,9 @@ def test_metrics_cpp():
     diff_and_sc = {
         'diff': '',
         'source_code': sc,
-        'source_code_before': sc
+        'source_code_before': sc,
+        'content': sc,
+        'content_before': sc
     }
 
     m1 = ModifiedFile('test-repos/lizard/FileCPP.cpp',
@@ -164,7 +171,9 @@ def test_metrics_java():
     diff_and_sc = {
         'diff': '',
         'source_code': sc,
-        'source_code_before': sc
+        'source_code_before': sc,
+        'content': sc,
+        'content_before': sc
     }
 
     m1 = ModifiedFile('test-repos/lizard/FileJava.java',
@@ -184,7 +193,9 @@ def test_metrics_not_supported_file():
     diff_and_sc = {
         'diff': '',
         'source_code': sc,
-        'source_code_before': sc
+        'source_code_before': sc,
+        'content': sc,
+        'content_before': sc
     }
 
     m1 = ModifiedFile('test-repos/lizard/NotSupported.pdf',
@@ -280,12 +291,56 @@ def test_tzoffset_plus_hours(repo: Git):
     assert tz2 == -7200  # +2 hours
 
 
+@pytest.mark.parametrize('repo', ['test-repos/complex_repo'], indirect=True)
+def test_content_before(repo: Git):
+    m1 = repo.get_commit('ffccf1e7497eb8136fd66ed5e42bef29677c4b71').modified_files[0]
+
+    assert m1.content is None
+    assert m1.content_before is not None
+
+
 @pytest.mark.parametrize('repo', ['test-repos/complex_repo'], indirect=True)
 def test_source_code_before(repo: Git):
     m1 = repo.get_commit('ffccf1e7497eb8136fd66ed5e42bef29677c4b71').modified_files[0]
 
-    assert m1.source_code is None
-    assert m1.source_code_before is not None
+    with catch_warnings(record=True) as w:
+        assert m1.source_code is None
+        assert len(w) == 1
+        assert issubclass(w[0].category, DeprecationWarning)
+
+    with catch_warnings(record=True) as w:
+        assert m1.source_code_before is not None
+        assert len(w) == 1
+        assert issubclass(w[0].category, DeprecationWarning)
+
+
+@pytest.mark.parametrize('repo', ['test-repos/source_code_before_commit'], indirect=True)
+def test_content_before_complete(repo: Git):
+    m1 = repo.get_commit('ca1f75455f064410360bc56218d0418221cf9484').modified_files[0]
+
+    with open('test-repos/source_code_before_commit/'
+              'sc_A_ca1f75455f064410360bc56218d0418221cf9484.txt') as f:
+        sc = f.read()
+
+    assert m1.content == sc
+    assert m1.content_before is None
+
+    old_sc = sc
+    with open(
+            'test-repos/source_code_before_commit/'
+            'sc_A_022ebf5fba835c6d95e99eaccc2d85b3db5a2ec0.txt') as f:
+        sc = f.read()
+
+    m1 = repo.get_commit('022ebf5fba835c6d95e99eaccc2d85b3db5a2ec0').modified_files[0]
+
+    assert m1.content == sc
+    assert m1.content_before == old_sc
+
+    old_sc = sc
+    m1 = repo.get_commit('ecd6780457835a2fc85c532338a29f2c98a6cfeb').modified_files[0]
+
+    assert m1.content is None
+    assert m1.content_before == old_sc
 
 
 @pytest.mark.parametrize('repo', ['test-repos/source_code_before_commit'], indirect=True)
@@ -296,8 +351,15 @@ def test_source_code_before_complete(repo: Git):
               'sc_A_ca1f75455f064410360bc56218d0418221cf9484.txt') as f:
         sc = f.read()
 
-    assert m1.source_code == sc
-    assert m1.source_code_before is None
+    with catch_warnings(record=True) as w:
+        assert m1.source_code == sc
+        assert len(w) == 1
+        assert issubclass(w[0].category, DeprecationWarning)
+
+    with catch_warnings(record=True) as w:
+        assert m1.source_code_before is None
+        assert len(w) == 1
+        assert issubclass(w[0].category, DeprecationWarning)
 
     old_sc = sc
     with open(
@@ -307,14 +369,28 @@ def test_source_code_before_complete(repo: Git):
 
     m1 = repo.get_commit('022ebf5fba835c6d95e99eaccc2d85b3db5a2ec0').modified_files[0]
 
-    assert m1.source_code == sc
-    assert m1.source_code_before == old_sc
+    with catch_warnings(record=True) as w:
+        assert m1.source_code == sc
+        assert len(w) == 1
+        assert issubclass(w[0].category, DeprecationWarning)
+
+    with catch_warnings(record=True) as w:
+        assert m1.source_code_before == old_sc
+        assert len(w) == 1
+        assert issubclass(w[0].category, DeprecationWarning)
 
     old_sc = sc
     m1 = repo.get_commit('ecd6780457835a2fc85c532338a29f2c98a6cfeb').modified_files[0]
 
-    assert m1.source_code is None
-    assert m1.source_code_before == old_sc
+    with catch_warnings(record=True) as w:
+        assert m1.source_code is None
+        assert len(w) == 1
+        assert issubclass(w[0].category, DeprecationWarning)
+
+    with catch_warnings(record=True) as w:
+        assert m1.source_code_before == old_sc
+        assert len(w) == 1
+        assert issubclass(w[0].category, DeprecationWarning)
 
 
 @pytest.mark.parametrize('repo', ['test-repos/small_repo'], indirect=True)
diff --git a/tests/test_git_repository.py b/tests/test_git_repository.py
index efc7e78..aaa370b 100644
--- a/tests/test_git_repository.py
+++ b/tests/test_git_repository.py
@@ -13,6 +13,7 @@
 # limitations under the License.
 from datetime import datetime, timezone, timedelta
 from pathlib import Path
+from warnings import catch_warnings
 
 import pytest
 from git import Git as PyGit
@@ -243,7 +244,12 @@ def test_should_detail_a_commit(repo: Git):
 
     assert commit.modified_files[0].new_path == "Matricula.java"
     assert commit.modified_files[0].diff.startswith("@@ -0,0 +1,62 @@\n+package model;") is True
-    assert commit.modified_files[0].source_code.startswith("package model;") is True
+    assert commit.modified_files[0].content.startswith("package model;") is True
+
+    with catch_warnings(record=True) as w:
+        assert commit.modified_files[0].source_code.startswith("package model;") is True
+        assert len(w) == 1
+        assert issubclass(w[0].category, DeprecationWarning)
 
 
 @pytest.mark.parametrize('repo', ['test-repos/branches_merged'], indirect=True)