Skip to content

Commit

Permalink
Merge branch 'master' of https://github.com/pyta-uoft/pyta into repor…
Browse files Browse the repository at this point in the history
…ting-errors-in-parsing-config
  • Loading branch information
sushimon committed Jul 8, 2023
2 parents 385942f + a062b0a commit 448ab5b
Show file tree
Hide file tree
Showing 11 changed files with 167 additions and 54 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,14 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
are missing.
- When running `check_contracts` on a class with type aliases as type annotations for its attributes, the `NameError`
that appears (which indicates that the type alias is undefined) is now resolved.
- The default value of `pyta-number-of-messages` is now 0. This automatically displays all occurrences of the same error.

### 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`.

### New checkers

Expand Down
2 changes: 1 addition & 1 deletion docs/usage/configuration.md
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# Configuration

```{note}
This page is current under construction!
This page is currently under construction!
```

## Providing Your Own Configuration Settings
Expand Down
2 changes: 1 addition & 1 deletion python_ta/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -309,7 +309,7 @@ def reset_linter(
(
"pyta-number-of-messages",
{
"default": 5,
"default": 0, # If the value is 0, all messages are displayed.
"type": "int",
"metavar": "<number_messages>",
"help": "Display a certain number of messages to the user, without overwhelming them.",
Expand Down
4 changes: 2 additions & 2 deletions python_ta/config/.pylintrc
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@

# Make sure to register custom options tuple first in `python_ta/__init__.py`
# ===========================================================
# Default max amount of messages for reporter to display.
pyta-number-of-messages = 5
# Default max amount of messages for reporter to display. If the value is 0, all messages are displayed.
pyta-number-of-messages = 0

# (DEPRECATED: Use output-format option below.) Set the [REPORTS] output-format option instead.
# pyta-reporter = HTMLReporter
Expand Down
18 changes: 17 additions & 1 deletion python_ta/reporters/json_reporter.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,24 @@ def display_messages(self, layout: BaseLayout) -> None:
output.append(
{
"filename": k,
"msgs": [msg.to_dict() for msg in msgs],
"msgs": self._output_messages(msgs),
}
)

self.writeln(json.dumps(output, indent=4))

def _output_messages(self, msgs: List[NewMessage]) -> List[Dict]:
"""Returns a list of dictionaries containing formatted error messages."""
max_messages = self.linter.config.pyta_number_of_messages
num_occurrences = {msg.message.msg_id: 0 for msg in msgs}
output_lst = []

for msg in msgs:
if max_messages == 0 or num_occurrences[msg.message.msg_id] < max_messages:
output_lst.append(msg.to_dict())
num_occurrences[msg.message.msg_id] += 1

for msg_dict in output_lst:
msg_dict["number_of_occurrences"] = num_occurrences[msg_dict["msg_id"]]

return output_lst
8 changes: 6 additions & 2 deletions python_ta/reporters/plain_reporter.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,12 +59,16 @@ def _colour_messages_by_type(self, messages: Dict[str, List[NewMessage]]) -> str
result += self._colourify("bold", msg_id)
result += self._colourify("bold", " ({}) ".format(messages[msg_id][0].symbol))
result += "Number of occurrences: {}.".format(len(messages[msg_id]))
if max_messages != float("inf") and max_messages < len(messages[msg_id]):
if (
max_messages != 0
and max_messages != float("inf")
and max_messages < len(messages[msg_id])
):
result += " (First {} shown).".format(max_messages)
result += self._BREAK

for i, msg in enumerate(messages[msg_id]):
if i == max_messages:
if max_messages != 0 and i == max_messages:
break

# Use only explanation, without redundant accessory information
Expand Down
93 changes: 48 additions & 45 deletions python_ta/reporters/templates/template.html.jinja
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@
{% endif %}
</span>
<span class="shown">
{% if limit < num_occurrences %}
{% if 0 < limit < num_occurrences %}
(First {{ limit }} shown).
{% endif %}
</span>
Expand All @@ -141,27 +141,29 @@
</div>
<div class="message-container collapsible expanded">
{% for indiv in occurrences %}
<div class="message">
<p class="message-name">
[Line {{ indiv.line }}] {{ indiv.msg }}
<span class="slider">
<button style="float: right;">
<svg class="collapse-trigger" viewBox="0 1 10 10" width="60" height="20">
<g transform="scale(0.13 0.13)">
<path d="M 10 30 l 40 40 l 40 -40" stroke="#777777" stroke-width="15px" fill="transparent"/>
</g>
</svg>
</button>
</span>
</p>
{% if indiv.snippet != '' %}
<div class="collapsible expanded message-snippet-container">
<pre class="message-snippet">{{ indiv.snippet }}</pre>
</div>
{% else %}
<span class="empty-placeholder">{{ reporter.no_snippet }}</span>
{% endif %}
</div>
{% if limit == 0 or loop.index0 < limit %}
<div class="message">
<p class="message-name">
[Line {{ indiv.line }}] {{ indiv.msg }}
<span class="slider">
<button style="float: right;">
<svg class="collapse-trigger" viewBox="0 1 10 10" width="60" height="20">
<g transform="scale(0.13 0.13)">
<path d="M 10 30 l 40 40 l 40 -40" stroke="#777777" stroke-width="15px" fill="transparent"/>
</g>
</svg>
</button>
</span>
</p>
{% if indiv.snippet != '' %}
<div class="collapsible expanded message-snippet-container">
<pre class="message-snippet">{{ indiv.snippet }}</pre>
</div>
{% else %}
<span class="empty-placeholder">{{ reporter.no_snippet }}</span>
{% endif %}
</div>
{% endif %}
{% endfor %}
</div>
</div>
Expand Down Expand Up @@ -201,7 +203,7 @@
{% endif %}
</span>
<span class="shown">
{% if limit < num_occurrences %}
{% if 0 < limit < num_occurrences %}
(First {{ limit }} shown).
{% endif %}
</span>
Expand All @@ -215,30 +217,31 @@
</button>
</span>
</div>

<div class="message-container collapsible expanded">
{% for indiv in occurrences %}
<div class="message">
<p class="message-name">
[Line {{ indiv.line }}] {{ indiv.msg }}
<span class="slider">
<button style="float: right;">
<svg class="collapse-trigger" viewBox="0 1 10 10" width="60" height="20">
<g transform="scale(0.13 0.13)">
<path d="M 10 30 l 40 40 l 40 -40" stroke="#777777" stroke-width="15px" fill="transparent"/>
</g>
</svg>
</button>
</span>
</p>
<div class="collapsible expanded message-snippet-container">
{% if indiv.snippet != '' %}
<pre class="message-snippet">{{ indiv.snippet }}</pre>
{% else %}
<span class="empty-placeholder">{{ reporter.no_snippet }}</span>
{% endif %}
</div>
</div>
{% if limit == 0 or loop.index0 < limit %}
<div class="message">
<p class="message-name">
[Line {{ indiv.line }}] {{ indiv.msg }}
<span class="slider">
<button style="float: right;">
<svg class="collapse-trigger" viewBox="0 1 10 10" width="60" height="20">
<g transform="scale(0.13 0.13)">
<path d="M 10 30 l 40 40 l 40 -40" stroke="#777777" stroke-width="15px" fill="transparent"/>
</g>
</svg>
</button>
</span>
</p>
<div class="collapsible expanded message-snippet-container">
{% if indiv.snippet != '' %}
<pre class="message-snippet">{{ indiv.snippet }}</pre>
{% else %}
<span class="empty-placeholder">{{ reporter.no_snippet }}</span>
{% endif %}
</div>
</div>
{% endif %}
{% endfor %}
</div>

Expand Down
2 changes: 1 addition & 1 deletion tests/test.pylintrc
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
# Make sure to register custom options tuple first in `python_ta/__init__.py`
# ===========================================================
# Default max amount of messages for reporter to display.
pyta-number-of-messages = 5
pyta-number-of-messages = 0

# (DEPRECATED: Use output-format option below.) Set the [REPORTS] output-format option instead.
# pyta-reporter = HTMLReporter
Expand Down
32 changes: 32 additions & 0 deletions tests/test_config/file_fixtures/funcs_with_errors.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
"""Python script used for testing that the correct number of error occurrences are being displayed."""
from typing import List

# The following imports are used solely to trigger errors.
import packaging
import pip
import pygments
import pylint


def sum_items(lst: List[int]) -> int:
"""..."""
s = 0
for i in range(len(lst)):
s += lst[i]
return s


def sum_items2(lst: List[int]) -> int:
"""..."""
s = 0
for i in range(0, len(lst)):
s += lst[i]
return s


def sum_items3(lst: List[int]) -> int:
"""..."""
s = 0
for i in range(0, len(lst), 1):
s += lst[i]
return s
55 changes: 55 additions & 0 deletions tests/test_config/test_num_error_occurrences.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
"""
Test suite for checking that the correct number of error occurrences are being displayed.
"""

import contextlib
import io
import os

from python_ta import check_all


def pyta_output(num_msgs: int) -> str:
"""Returns the PythonTA report as a string."""
output = io.StringIO()

curr_dir = os.path.dirname(__file__)
test_file = os.path.join(curr_dir, "file_fixtures", "funcs_with_errors.py")

with contextlib.redirect_stdout(output):
check_all(
module_name=test_file,
config={
"pyta-number-of-messages": num_msgs,
"output-format": "python_ta.reporters.JSONReporter",
},
)

return output.getvalue()


def test_default() -> None:
"""Tests that all messages are displayed when pyta-number-of-messages = 0."""
pyta_report = pyta_output(0)
expected = 12
actual = pyta_report.count("msg_id")

assert expected == actual


def test_num_msgs2() -> None:
"""Tests that only two messages per error are displayed when pyta-number-of-messages = 2."""
pyta_report = pyta_output(2)
expected = 7
actual = pyta_report.count("msg_id")

assert expected == actual


def test_num_msgs_greater() -> None:
"""Tests that all messages are displayed when pyta-number-of-messages is greater than the number of errors."""
pyta_report = pyta_output(5)
expected = 12
actual = pyta_report.count("msg_id")

assert expected == actual
2 changes: 1 addition & 1 deletion tests/test_messages_config/test.pylintrc
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
# Make sure to register custom options tuple first in `python_ta/__init__.py`
# ===========================================================
# Default max amount of messages for reporter to display.
pyta-number-of-messages = 5
pyta-number-of-messages = 0

# (DEPRECATED: Use output-format option below.) Set the [REPORTS] output-format option instead.
# pyta-reporter = HTMLReporter
Expand Down

0 comments on commit 448ab5b

Please sign in to comment.