Fixed PythonTA crash when reporting error messages in config files (#920

)

CHANGELOG.md

@@ -30,6 +30,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 ### Bug Fixes
 
 - Fixed bug where running `python3 -m python_ta --generate-config` yields a `FileNotFoundError`.
+- Fixed bug in how PythonTA reports error messages that occur when parsing configuration files.
 - Fixed bug where the HTML reporter would display all error occurrences of the same type despite stating that only a limited number was being shown.
 - Fixed bug where the JSON reporter was not limiting the number of error occurrences displayed with respect to `pyta-number-of-messages`.
 

python_ta/__init__.py

@@ -138,8 +138,6 @@ def _check(
             for file_py in get_file_paths(locations):
                 if not _verify_pre_check(file_py):
                     continue  # Check the other files
-                # The current file can actually be checked so update the flag
-                is_any_file_checked = True
                 # Load config file in user location. Construct new linter each
                 # time, so config options don't bleed to unintended files.
                 # Reuse the same reporter each time to accumulate the results across different files.
@@ -148,7 +146,22 @@ def _check(
                     file_linted=file_py,
                     load_default_config=load_default_config,
                 )
-                linter.set_reporter(current_reporter)
+
+                if not is_any_file_checked:
+                    prev_output = current_reporter.out
+                    current_reporter = linter.reporter
+                    current_reporter.out = prev_output
+
+                    # At this point, the only possible errors are those from parsing the config file
+                    # so print them, if there are any.
+                    if current_reporter.messages:
+                        current_reporter.print_messages()
+                else:
+                    linter.set_reporter(current_reporter)
+
+                # The current file was checked so update the flag
+                is_any_file_checked = True
+
                 module_name = os.path.splitext(os.path.basename(file_py))[0]
                 if module_name in MANAGER.astroid_cache:  # Remove module from astroid cache
                     del MANAGER.astroid_cache[module_name]
@@ -221,6 +234,8 @@ def _override_config(linter: PyLinter, config_location: AnyStr) -> None:
 
     Snippets taken from pylint.config.config_initialization.
     """
+    linter.set_current_module(config_location)
+
     # Read the configuration file.
     config_file_parser = _ConfigurationFileParser(verbose=True, linter=linter)
     try:
@@ -235,6 +250,9 @@ def _override_config(linter: PyLinter, config_location: AnyStr) -> None:
     except _UnrecognizedOptionError as exc:
         print(f"Unrecognized options: {', '.join(exc.options)}", file=sys.stderr)
 
+    # Everything has been set up already so emit any stashed messages.
+    linter._emit_stashed_messages()
+
     linter.config_file = config_location
 
 

python_ta/reporters/core.py

@@ -3,6 +3,7 @@
 import os.path
 import sys
 from collections import defaultdict
+from pathlib import Path
 from typing import Dict, List, Optional, Tuple
 
 from astroid import NodeNG
@@ -40,7 +41,10 @@ def to_dict(self) -> Dict:
 
 
 # Messages without a source code line to highlight
-NO_SNIPPET = {"invalid-name"}
+NO_SNIPPET = {
+    "invalid-name",
+    "unknown-option-value",
+}
 
 
 class PythonTaReporter(BaseReporter):
@@ -215,6 +219,13 @@ def _colourify(cls, colour_class: str, text: str) -> str:
     # Event callbacks
     def on_set_current_module(self, module: str, filepath: Optional[str]) -> None:
         """Hook called when a module starts to be analysed."""
+        # First, check if `module` is the name of a config file and if so, make filepath the
+        # corresponding path to that config file.
+        possible_config_path = Path(os.path.expandvars(module)).expanduser()
+
+        if possible_config_path.exists() and filepath is None:
+            filepath = str(possible_config_path)
+
         # Skip if filepath is None
         if filepath is None:
             return

tests/test_config/file_fixtures/test_json_with_errors.pylintrc

@@ -0,0 +1,16 @@
+[REPORTS]
+
+output-format = python_ta.reporters.JSONReporter
+
+[FORMAT]
+
+max-line-length = 100
+
+ignore-long-lines = ^\s*((# )?<?https?://\S+>?)|(>>>.*)$
+
+[FORBIDDEN IMPORT]
+extra-imports = math, tkinter
+
+[MESSAGES CONTROL]
+disable =
+    ooga

tests/test_config/file_fixtures/test_plain_with_errors.pylintrc

@@ -0,0 +1,16 @@
+[REPORTS]
+
+output-format = python_ta.reporters.PlainReporter
+
+[FORMAT]
+
+max-line-length = 100
+
+ignore-long-lines = ^\s*((# )?<?https?://\S+>?)|(>>>.*)$
+
+[FORBIDDEN IMPORT]
+extra-imports = math, tkinter
+
+[MESSAGES CONTROL]
+disable =
+    ooga

tests/test_config/file_fixtures/test_with_errors.pylintrc

@@ -0,0 +1,16 @@
+[REPORTS]
+
+output-format = python_ta.reporters.HTMLReporter
+
+[FORMAT]
+
+max-line-length = 100
+
+ignore-long-lines = ^\s*((# )?<?https?://\S+>?)|(>>>.*)$
+
+[FORBIDDEN IMPORT]
+extra-imports = math, tkinter
+
+[MESSAGES CONTROL]
+disable =
+    ooga

tests/test_config/test_config.py

@@ -1,6 +1,7 @@
 """
 Test suite for checking whether configuration worked correctly with user-inputted configurations.
 """
+import json
 import os
 
 import pytest
@@ -121,3 +122,59 @@ def test_default_pylint_checks_in_no_default(configure_linter_no_default) -> Non
     assert all(
         check not in disabled_checks for check in previously_disabled_checks if disabled_checks
     )
+
+
+def test_unknown_option_value() -> None:
+    """Test that the configuration options gets overridden without error when there is an unknown
+    option value."""
+    curr_dir = os.path.dirname(__file__)
+    config = os.path.join(curr_dir, "file_fixtures", "test_with_errors.pylintrc")
+    reporter = python_ta.reset_linter(config=config).reporter
+
+    # Check if there are any messages with the msg_id `W0012` (the code corresponding to the error
+    # `unknown-option-value`.
+    message_ids = [msg.msg_id for message_lis in reporter.messages.values() for msg in message_lis]
+
+    assert "W0012" in message_ids
+
+
+def test_unknown_option_value_no_default() -> None:
+    """Test that the configuration options gets loaded without error when there is an unknown option
+    value.
+
+    The default options are not loaded from the PythonTA default config."""
+    curr_dir = os.path.dirname(__file__)
+    config = os.path.join(curr_dir, "file_fixtures", "test_with_errors.pylintrc")
+    reporter = python_ta.reset_linter(config=config, load_default_config=False).reporter
+
+    # Check if there are any messages with the msg_id `W0012` (the code corresponding to the error
+    # `unknown-option-value`.
+    message_ids = [msg.msg_id for message_lis in reporter.messages.values() for msg in message_lis]
+
+    assert "W0012" in message_ids
+
+
+def test_json_config_parsing_error(capsys) -> None:
+    """Test that the JSONReporter correctly includes the config parsing error in its report."""
+    curr_dir = os.path.dirname(__file__)
+    config = os.path.join(curr_dir, "file_fixtures", "test_json_with_errors.pylintrc")
+    python_ta.check_all(module_name="examples/nodes/name.py", config=config)
+    out = capsys.readouterr().out
+
+    reports = json.loads(out)
+
+    assert any(msg["msg_id"] == "W0012" for report in reports for msg in report["msgs"])
+
+
+def test_print_messages_config_parsing_error(capsys) -> None:
+    """Test that print_messages correctly prints the config parsing error in its report.
+
+    This tests the functionality of PlainReporter and ColorReporter to verify it correctly includes
+    the config parsing error in its printed report."""
+    curr_dir = os.path.dirname(__file__)
+    config = os.path.join(curr_dir, "file_fixtures", "test_plain_with_errors.pylintrc")
+    python_ta.check_all(module_name="examples/nodes/name.py", config=config)
+
+    out = capsys.readouterr().out
+
+    assert "W0012" in out