Skip to content

Commit

Permalink
Removed code snippet in the error message for config file parsing err…
Browse files Browse the repository at this point in the history
…ors (#927)
  • Loading branch information
sushimon authored Jul 14, 2023
1 parent 43240cf commit 2109a69
Show file tree
Hide file tree
Showing 6 changed files with 86 additions and 14 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
- 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`.
- Ensured some config file parsing errors no longer display incorrect lines of code as the source of the error.

### New checkers

Expand Down
3 changes: 2 additions & 1 deletion python_ta/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,8 @@ def _override_config(linter: PyLinter, config_location: AnyStr) -> None:
try:
linter._parse_configuration_file(config_args)
except _UnrecognizedOptionError as exc:
print(f"Unrecognized options: {', '.join(exc.options)}", file=sys.stderr)
unrecognized_options_message = ", ".join(exc.options)
linter.add_message("unrecognized-option", args=unrecognized_options_message, line=0)

# Everything has been set up already so emit any stashed messages.
linter._emit_stashed_messages()
Expand Down
3 changes: 3 additions & 0 deletions python_ta/reporters/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,9 @@ def to_dict(self) -> Dict:
NO_SNIPPET = {
"invalid-name",
"unknown-option-value",
"config-parse-error",
"useless-option-value",
"unrecognized-option",
}


Expand Down
15 changes: 15 additions & 0 deletions tests/test_config/file_fixtures/test_f0011.pylintrc
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
[REPORTS]

output-format = python_ta.reporters.HTMLReporter

[FORMAT]

max-line-length = 100

ignore-long-lines = ^\s*((# )?<?https?://\S+>?)|(>>>.*)$

[FORBIDDEN IMPORT]
extra-imports = math, tkinter

# To trigger F0011
bad
6 changes: 5 additions & 1 deletion tests/test_config/file_fixtures/test_with_errors.pylintrc
Original file line number Diff line number Diff line change
Expand Up @@ -12,5 +12,9 @@ ignore-long-lines = ^\s*((# )?<?https?://\S+>?)|(>>>.*)$
extra-imports = math, tkinter

[MESSAGES CONTROL]
# To trigger W0012, R0022
disable =
ooga
ooga, bad-continuation

# To trigger E0015
no-space-check =
72 changes: 60 additions & 12 deletions tests/test_config/test_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,13 @@
"max-line-length": 120,
}

# Non-fatal config errors. Fatal errors will be checked individually.
CONFIG_ERRORS_TO_CHECK = {
"W0012",
"R0022",
"E0015",
}


@pytest.fixture
def configure_linter_load_default():
Expand Down Expand Up @@ -124,34 +131,36 @@ def test_default_pylint_checks_in_no_default(configure_linter_no_default) -> Non
)


def test_unknown_option_value() -> None:
"""Test that the configuration options gets overridden without error when there is an unknown
option value."""
def test_config_parsing_errors() -> None:
"""Test that the configuration options gets overridden without error when there are errors
parsing the config files.
This checks the non-fatal errors from parsing the config file."""
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`.
# Check if there are messages with `msg_id`s from CONFIG_ERRORS_TO_CHECK.
message_ids = [msg.msg_id for message_lis in reporter.messages.values() for msg in message_lis]

assert "W0012" in message_ids
assert all(error in message_ids for error in CONFIG_ERRORS_TO_CHECK)


def test_unknown_option_value_no_default() -> None:
"""Test that the configuration options gets loaded without error when there is an unknown option
value.
def test_config_parsing_errors_no_default() -> None:
"""Test that the configuration options gets loaded without error when there are errors
parsing the config files.
This checks the non-fatal errors from parsing the config file.
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`.
# Check if there are messages with `msg_id`s from CONFIG_ERRORS_TO_CHECK.
message_ids = [msg.msg_id for message_lis in reporter.messages.values() for msg in message_lis]

assert "W0012" in message_ids
assert all(error in message_ids for error in CONFIG_ERRORS_TO_CHECK)


def test_json_config_parsing_error(capsys) -> None:
Expand All @@ -178,3 +187,42 @@ def test_print_messages_config_parsing_error(capsys) -> None:
out = capsys.readouterr().out

assert "W0012" in out


def test_no_snippet_for_config_parsing_errors() -> None:
"""Test that there is no snippet being built for errors that come from parsing the config file."""
curr_dir = os.path.dirname(__file__)
config = os.path.join(curr_dir, "file_fixtures", "test_with_errors.pylintrc")
reporter = python_ta.check_all(module_name="examples/nodes/name.py", config=config)

# Gather all the built code snippets for the `msg_id`s specified in CONFIG_ERRORS_TO_CHECK.
snippets = [
msg.snippet
for message_lis in reporter.messages.values()
for msg in message_lis
if msg.msg_id in CONFIG_ERRORS_TO_CHECK
]

assert all(snippet == "" for snippet in snippets)


def test_config_parse_error(capsys) -> None:
"""Test that F0011 (config-parse-error) correctly gets reported."""
curr_dir = os.path.dirname(__file__)
config = os.path.join(curr_dir, "file_fixtures", "test_f0011.pylintrc")
reporter = python_ta.check_all(module_name="examples/nodes/name.py", config=config)

msg_id = reporter.messages[config][0].msg_id

assert msg_id == "F0011"


def test_config_parse_error_has_no_snippet() -> None:
"""Test that F0011 (config-parse-error) correctly gets reported with no code snippet."""
curr_dir = os.path.dirname(__file__)
config = os.path.join(curr_dir, "file_fixtures", "test_f0011.pylintrc")
reporter = python_ta.check_all(module_name="examples/nodes/name.py", config=config)

snippet = reporter.messages[config][0].snippet

assert snippet == ""

0 comments on commit 2109a69

Please sign in to comment.