Merge branch 'master' of https://github.com/pyta-uoft/pyta into repor…

…ting-errors-in-parsing-config
pyta-uoft · Jul 8, 2023 · 1b643f2 · 1b643f2
2 parents 2cff098 + a062b0a
commit 1b643f2
Show file tree

Hide file tree

Showing 11 changed files with 167 additions and 54 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -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
 

diff --git a/docs/usage/configuration.md b/docs/usage/configuration.md
@@ -1,7 +1,7 @@
 # Configuration
 
 ```{note}
-This page is current under construction!
+This page is currently under construction!
 ```
 
 ## Providing Your Own Configuration Settings

diff --git a/python_ta/__init__.py b/python_ta/__init__.py
@@ -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.",

diff --git a/python_ta/config/.pylintrc b/python_ta/config/.pylintrc
@@ -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

diff --git a/python_ta/reporters/json_reporter.py b/python_ta/reporters/json_reporter.py
@@ -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
diff --git a/python_ta/reporters/plain_reporter.py b/python_ta/reporters/plain_reporter.py
@@ -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

diff --git a/python_ta/reporters/templates/template.html.jinja b/python_ta/reporters/templates/template.html.jinja
@@ -125,7 +125,7 @@
                                     {% endif %}
                                     </span>
                                     <span class="shown">
-                                    {% if limit < num_occurrences %}
+                                    {% if 0 < limit < num_occurrences %}
                                      (First {{ limit }} shown).
                                     {% endif %}
                                     </span>
@@ -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>
@@ -201,7 +203,7 @@
                                         {% endif %}
                                     </span>
                                     <span class="shown">
-                                    {% if limit < num_occurrences %}
+                                    {% if 0 < limit < num_occurrences %}
                                      (First {{ limit }} shown).
                                     {% endif %}
                                     </span>
@@ -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>
 

diff --git a/tests/test.pylintrc b/tests/test.pylintrc
@@ -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

diff --git a/tests/test_config/file_fixtures/funcs_with_errors.py b/tests/test_config/file_fixtures/funcs_with_errors.py
@@ -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
diff --git a/tests/test_config/test_num_error_occurrences.py b/tests/test_config/test_num_error_occurrences.py
@@ -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
diff --git a/tests/test_messages_config/test.pylintrc b/tests/test_messages_config/test.pylintrc
@@ -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