Refactoring

.github/pull_request_template.md

@@ -1,5 +1,5 @@
 **Issues**:
-[#ALT-](https://vyahhi.myjetbrains.com/youtrack/issue/ALT-)
+[Issue#](https://github.com/hyperskill/hyperstyle/issues/#)
 
 **Description**:
 

src/python/review/common/file_system.py

@@ -2,30 +2,52 @@
 import os
 import tempfile
 from contextlib import contextmanager
+from enum import Enum
 from pathlib import Path
-from typing import List, Union
+from typing import List, Union, Callable
 
 
-# TODO: Need testing
-def get_all_file_paths_in_dir(dir_path: Path) -> List[Path]:
-    if not dir_path.is_dir():
-        raise ValueError
+class FileSystemItem(Enum):
+    PATH = 0
+    SUBDIR = 1
+    FILE = 2
 
-    file_paths = []
-    for root, _, files in os.walk(dir_path):
-        for file in files:
-            file_paths.append(Path(root) / file)
 
-    return file_paths
+class Encoding(Enum):
+    ISO_ENCODING = 'ISO-8859-1'
+    UTF_ENCODING = 'utf-8'
 
 
-# TODO: Need testing
-def get_all_subdirs(dir_path: Path) -> List[Path]:
-    subdirs = {dir_path}
-    for file_path in get_all_file_paths_in_dir(dir_path):
-        subdirs.add(file_path.parent)
+# Make sure all extensions (except an empty one) have a dot
+class Extension(Enum):
+    EMPTY = ''
+    PY = '.py'
+    JAVA = '.java'
+    KT = '.kt'
+    JS = '.js'
+    KTS = '.kts'
+
+
+ItemCondition = Callable[[str], bool]
+
+
+def all_items_condition(name: str) -> bool:
+    return True
+
 
-    return list(subdirs)
+# To get all files or subdirs (depends on the last parameter) from root that match item_condition
+# Note that all subdirs or files already contain the full path for them
+def get_all_file_system_items(root: Path, item_condition: ItemCondition = all_items_condition,
+                              item_type: FileSystemItem = FileSystemItem.FILE) -> List[Path]:
+    if not root.is_dir():
+        raise ValueError(f'The {root} is not a directory')
+
+    items = []
+    for fs_tuple in os.walk(root):
+        for item in fs_tuple[item_type.value]:
+            if item_condition(item):
+                items.append(Path(os.path.join(fs_tuple[FileSystemItem.PATH.value], item)))
+    return items
 
 
 # TODO: Need testing
@@ -35,22 +57,34 @@ def new_temp_dir() -> Path:
         yield Path(temp_dir)
 
 
-def write(path: Union[str, Path], text: str):
-    """
-    Write text to file. Create all parents if necessary
-    :param path:
-    :param text:
-    :return:
-    """
-    path = Path(path)
+# File should contain the full path and its extension.
+# Create all parents if necessary
+def create_file(file_path: Union[str, Path], content: str):
+    file_path = Path(file_path)
+
+    create_directory(os.path.dirname(file_path))
+    with open(file_path, 'w') as f:
+        f.write(content)
 
-    os.makedirs(path.parent, exist_ok=True)
-    with open(str(path), 'w') as file:
-        file.write(text)
+
+def create_directory(directory: str) -> None:
+    os.makedirs(directory, exist_ok=True)
 
 
 def get_file_line(path: Path, line_number: int):
     return linecache.getline(
         str(path),
         line_number
     ).strip()
+
+
+def get_content_from_file(file_path: Path, encoding: str = Encoding.ISO_ENCODING.value, to_strip_nl: bool = True) -> str:
+    with open(file_path, 'r', encoding=encoding) as f:
+        content = f.read()
+        return content if not to_strip_nl else content.rstrip('\n')
+
+
+# Not empty extensions are returned with a dot, for example, '.txt'
+# If file has no extensions, an empty one ('') is returned
+def get_extension_from_file(file: Path) -> Extension:
+    return Extension(os.path.splitext(file)[1])

src/python/review/common/java_compiler.py

@@ -3,12 +3,14 @@
 from pathlib import Path
 from typing import Union
 
+from src.python.review.common.file_system import Encoding, Extension
+
 logger = logging.getLogger(__name__)
 
 
 # TODO it cannot compile gradle-based projects
 def javac_project(dir_path: Path) -> bool:
-    return javac(f'$(find {dir_path} -name "*.java")')
+    return javac(f'$(find {dir_path} -name "*{Extension.JAVA.value}")')
 
 
 def javac(javac_args: Union[str, Path]) -> bool:
@@ -18,13 +20,13 @@ def javac(javac_args: Union[str, Path]) -> bool:
             shell=True,
             stderr=subprocess.STDOUT
         )
-        output_str = str(output_bytes, 'utf-8')
+        output_str = str(output_bytes, Encoding.UTF_ENCODING.value)
 
         if output_str:
             logger.debug(output_str)
 
         return True
     except subprocess.CalledProcessError as error:
         logger.error(f'Failed compile java code with error: '
-                     f'{str(error.stdout, "utf-8")}')
+                     f'{str(error.stdout, Encoding.UTF_ENCODING.value)}')
         return False

src/python/review/common/language.py

@@ -2,6 +2,8 @@
 from pathlib import Path
 from typing import List
 
+from src.python.review.common.file_system import Extension, get_extension_from_file
+
 
 @unique
 class Language(Enum):
@@ -13,19 +15,19 @@ class Language(Enum):
 
 
 EXTENSION_TO_LANGUAGE = {
-    '.java': Language.JAVA,
-    '.py': Language.PYTHON,
-    '.kt': Language.KOTLIN,
-    '.kts': Language.KOTLIN,
-    '.js': Language.JS,
+    Extension.JAVA: Language.JAVA,
+    Extension.PY: Language.PYTHON,
+    Extension.KT: Language.KOTLIN,
+    Extension.KTS: Language.KOTLIN,
+    Extension.JS: Language.JS,
 }
 
 
 def guess_file_language(file_path: Path) -> Language:
-    return EXTENSION_TO_LANGUAGE.get(file_path.suffix, Language.UNKNOWN)
+    return EXTENSION_TO_LANGUAGE.get(get_extension_from_file(file_path), Language.UNKNOWN)
 
 
-def filter_paths(file_paths: List[Path], language: Language) -> List[Path]:
+def filter_paths_by_language(file_paths: List[Path], language: Language) -> List[Path]:
     result = []
     for path in file_paths:
         if guess_file_language(path) == language:

src/python/review/common/parallel_runner.py

@@ -25,10 +25,7 @@ def run_inspector(path: Path,
 def inspect_in_parallel(path: Path,
                         config: ApplicationConfig,
                         inspectors: List[BaseInspector]) -> List[BaseIssue]:
-    inspectors = filter(
-        lambda inspector: inspector.inspector_type not in config.disabled_inspectors,
-        inspectors
-    )
+    inspectors = filter(lambda i: i.inspector_type not in config.disabled_inspectors, inspectors)
 
     if config.n_cpu == 1:
         issues = []

src/python/review/common/subprocess_runner.py

@@ -5,7 +5,7 @@
 logger = logging.getLogger(__name__)
 
 
-def run_in_subprocess_return_code(command: List[str]) -> (int, str):
+def run_in_subprocess(command: List[str]) -> str:
     process = subprocess.run(
         command,
         stdout=subprocess.PIPE,
@@ -20,8 +20,4 @@ def run_in_subprocess_return_code(command: List[str]) -> (int, str):
     if stderr:
         logger.debug('%s\'s stderr:\n%s' % (command[0], stderr))
 
-    return process.returncode, stdout
-
-
-def run_in_subprocess(command: List[str]) -> str:
-    return run_in_subprocess_return_code(command)[1]
+    return stdout

src/python/review/inspectors/base_inspector.py

@@ -7,12 +7,28 @@
 
 
 class BaseInspector(abc.ABC):
+    """
+    Each external inspector contains a dictionary in which the IssueType corresponds to the original linter classes.
+    The dictionary helps to categorize errors during parsing the linters' output.
 
+    To add a new inspector, you need:
+     - to create a class that inherits from the BaseInspector class,
+     - define the type of inspector (the type filed) by adding a new option in the InspectorType,
+     - implement the <inspect >function.
+
+    Typically, the <inspect> function launches a linter and parses its output (XML or JSON) to get a list of BaseIssue.
+
+    Some inspectors (internal) do not require creating a dictionary with IssueType.
+    This is connected to the fact that they do not launch an additional analysis tool and work with the code directly,
+    for example, the python AST inspector.
+    """
+
+    # Type of inspection for analyzing, e.g. pylint, detekt and etc
     @property
     @abc.abstractmethod
     def inspector_type(self) -> InspectorType:
-        raise NotImplementedError
+        raise NotImplementedError(f'inspector_type property not implemented yet')
 
     @abc.abstractmethod
     def inspect(self, path: Path, config: dict) -> List[BaseIssue]:
-        raise NotImplementedError
+        raise NotImplementedError(f'inspect method not implemented yet')

src/python/review/inspectors/flake8/flake8.py

@@ -7,7 +7,7 @@
 from src.python.review.inspectors.base_inspector import BaseInspector
 from src.python.review.inspectors.flake8.issue_types import CODE_PREFIX_TO_ISSUE_TYPE, CODE_TO_ISSUE_TYPE
 from src.python.review.inspectors.inspector_type import InspectorType
-from src.python.review.inspectors.issue import BaseIssue, CodeIssue, CyclomaticComplexityIssue, IssueType
+from src.python.review.inspectors.issue import BaseIssue, CodeIssue, CyclomaticComplexityIssue, IssueType, IssueData
 from src.python.review.inspectors.tips import get_cyclomatic_complexity_tip
 
 logger = logging.getLogger(__name__)
@@ -45,13 +45,9 @@ def parse(cls, output: str) -> List[BaseIssue]:
             file_path = Path(groups[0])
             line_no = int(groups[1])
 
-            issue_data = {
-                'file_path': file_path,
-                'line_no': line_no,
-                'column_no': int(groups[2]) if int(groups[2]) > 0 else 1,
-                'origin_class': origin_class,
-                'inspector_type': cls.inspector_type,
-            }
+            issue_data = IssueData.get_base_issue_data_dict(file_path, cls.inspector_type, line_number=line_no,
+                                                            column_number=int(groups[2]) if int(groups[2]) > 0 else 1,
+                                                            origin_class=origin_class)
             if cc_match is not None:
                 issue_data['description'] = get_cyclomatic_complexity_tip()
                 issue_data['cc_value'] = int(cc_match.groups()[1])

src/python/review/inspectors/intellij/intellij.py

@@ -5,7 +5,7 @@
 from typing import Dict, List, Optional, Union
 from xml.etree import ElementTree
 
-from src.python.review.common.file_system import get_all_file_paths_in_dir, new_temp_dir
+from src.python.review.common.file_system import get_all_file_system_items, new_temp_dir
 from src.python.review.common.subprocess_runner import run_in_subprocess
 from src.python.review.inspectors.base_inspector import BaseInspector
 from src.python.review.inspectors.inspector_type import InspectorType
@@ -71,7 +71,7 @@ def copy_files_to_project(self, path: Path) -> Dict[Path, Path]:
             file_paths = [path]
         elif path.is_dir():
             root_path = path
-            file_paths = get_all_file_paths_in_dir(root_path)
+            file_paths = get_all_file_system_items(root_path)
         else:
             raise ValueError
 
@@ -113,7 +113,7 @@ def _get_file_path_in_project(cls, relative_file_path: Path) -> Path:
     def parse(cls, out_dir_path: Path,
               path_in_project_to_origin_path: Dict[Path, Path]) -> List[BaseIssue]:
         out_file_paths = [
-            file_path for file_path in get_all_file_paths_in_dir(out_dir_path)
+            file_path for file_path in get_all_file_system_items(out_dir_path)
             if file_path.suffix.endswith('.xml') and not file_path.name.startswith('.')
         ]