From 3a5a358556d175eb9b295193a4533433a6e69c96 Mon Sep 17 00:00:00 2001
From: manderj <joffrey.mander+pro@gmail.com>
Date: Sun, 12 Jul 2020 14:57:11 +0200
Subject: [PATCH 01/11] Add new checker

---
 ChangeLog                                     |  4 +
 pylint/checkers/refactoring.py                | 83 +++++++++++++++++
 .../c/consider_using_min_max_builtin.py       | 89 +++++++++++++++++++
 .../c/consider_using_min_max_builtin.txt      | 11 +++
 4 files changed, 187 insertions(+)
 create mode 100644 tests/functional/c/consider_using_min_max_builtin.py
 create mode 100644 tests/functional/c/consider_using_min_max_builtin.txt

diff --git a/ChangeLog b/ChangeLog
index 1972552751..c8d5f5b917 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -7,6 +7,10 @@ What's New in Pylint 2.6.0?
 
 Release date: TBA
 
+* Add `consider-using-min-max-builtin` check for if statement which could be replaced by Python builtin min or max
+
+  Close #3406
+
 * Fix various scope-related bugs in ``undefined-variable`` checker
 
   Close #1082, #3434, #3461
diff --git a/pylint/checkers/refactoring.py b/pylint/checkers/refactoring.py
index 3af8709bf6..70dcd01482 100644
--- a/pylint/checkers/refactoring.py
+++ b/pylint/checkers/refactoring.py
@@ -316,6 +316,16 @@ class RefactoringChecker(checkers.BaseTokenChecker):
             "Emitted when calling the super() builtin with the current class "
             "and instance. On Python 3 these arguments are the default and they can be omitted.",
         ),
+        "R1726": (
+            'Consider using "%s" instead of the unnecessary if block',
+            "consider-using-min-builtin",
+            "It increase readability to use the min builtin",
+        ),
+        "R1727": (
+            'Consider using "%s" instead of the unnecessary if block',
+            "consider-using-max-builtin",
+            "It increase readability to use the max builtin",
+        ),
     }
     options = (
         (
@@ -633,6 +643,79 @@ def visit_if(self, node):
         self._check_superfluous_else_break(node)
         self._check_superfluous_else_continue(node)
         self._check_consider_get(node)
+        self._check_consider_using_min_max_builtin(node)
+
+    def _check_consider_using_min_max_builtin(self, node):
+        """Check if the given if node can be refactored as an min/max python builtin.
+        """
+        if self._is_actual_elif(node) or node.orelse:
+            # Not interested in if statements with multiple branches.
+            return
+
+        if len(node.body) != 1:
+            return
+
+        body = node.body[0]
+        # Check if condition can be reduced.
+        if not hasattr(body, 'targets') or len(body.targets) != 1:
+            return
+
+        target = body.targets[0]
+        if not(
+            isinstance(node.test, astroid.Compare)
+            and not isinstance(target, astroid.Subscript)
+            and not isinstance(node.test.left, astroid.Subscript)
+            and isinstance(body, astroid.Assign)
+        ):
+            return
+
+        # Check that the assignation is on the same variable.
+        # TODO: retrieve full class name
+        #import ipdb; ipdb.set_trace()
+        if hasattr(node.test.left, 'name'):
+            left_operand = node.test.left.name
+        elif hasattr(node.test.left, 'attrname'):
+            left_operand = node.test.left.attrname
+        else:
+            return
+
+        if hasattr(target, 'name'):
+            target_assignation = target.name
+        elif hasattr(target, 'attrname'):
+            target_assignation = target.attrname
+        else:
+            return
+
+        if not (left_operand == target_assignation):
+            return
+
+        if len(node.test.ops) > 1:
+            return
+
+        if not isinstance(body.value, (astroid.Name, astroid.Const)):
+            return
+
+        operator, right_statement = node.test.ops[0]
+        if isinstance(body.value, astroid.Name):
+            body_value = body.value.name
+        else:
+            body_value = body.value.value
+
+        if isinstance(right_statement, astroid.Name):
+            right_statement_value = right_statement.name
+        else:
+            right_statement_value = right_statement.value
+
+        # Verify the right part of the statement is the same.
+        if right_statement_value != body_value:
+            return
+
+        if operator in ("<", "<="):
+            reduced_to = "{target} = max({target}, {item})".format(target=target_assignation, item=body_value)
+            self.add_message("consider-using-max-builtin", node=node, args=(reduced_to,))
+        elif operator in (">", ">="):
+            reduced_to = "{target} = min({target}, {item})".format(target=target_assignation, item=body_value)
+            self.add_message("consider-using-min-builtin", node=node, args=(reduced_to,))
 
     @utils.check_messages("simplifiable-if-expression")
     def visit_ifexp(self, node):
diff --git a/tests/functional/c/consider_using_min_max_builtin.py b/tests/functional/c/consider_using_min_max_builtin.py
new file mode 100644
index 0000000000..8d573dde0c
--- /dev/null
+++ b/tests/functional/c/consider_using_min_max_builtin.py
@@ -0,0 +1,89 @@
+# pylint: disable=missing-docstring, invalid-name, too-few-public-methods, redefined-outer-name
+
+# value = 10
+# value2 = 0
+# value3 = 3
+
+# # Positive
+# if value < 10: # [consider-using-max-builtin]
+#     value = 10
+
+# if value >= 10: # [consider-using-min-builtin]
+#     value = 10
+
+# if value <= 10: # [consider-using-max-builtin]
+#     value = 10
+
+# if value > 10: # [consider-using-min-builtin]
+#     value = 10
+
+# if value < value2: # [consider-using-max-builtin]
+#     value = value2
+
+# if value > value2: # [consider-using-min-builtin]
+#     value = value2
+
+class A:
+    def __init__(self):
+        self.value = 13
+
+A1 = A()
+if A1.value > 10: # [consider-using-min-builtin]
+    A1.value = 10
+
+# class AA:
+#     def __init__(self, value):
+#         self.value = value
+#     def __gt__(self, b):
+#         return self.value > b
+#     def __ge__(self, b):
+#         return self.value >= b
+#     def __lt__(self, b):
+#         return self.value < b
+#     def __le__(self, b):
+#         return self.value <= b
+
+# A1 = AA(0)
+# A2 = AA(3)
+
+# if A1 > A2: # [consider-using-min-builtin]
+#     A1 = A2
+
+# if A2 < A1: # [consider-using-max-builtin]
+#     A2 = A1
+
+# if A1 >= A2: # [consider-using-min-builtin]
+#     A1 = A2
+
+# if A2 <= A1: # [consider-using-max-builtin]
+#     A2 = A1
+
+# # Negative
+# if value > 10:
+#     value = 2
+
+# if value > 10:
+#    value = 2
+#    value2 = 3
+
+# if value > value2:
+#     value = value3
+
+# if value > 5:
+#     value = value3
+
+# if 2 < value <= 3:
+#     value = 1
+
+# if value <= 3:
+#     value = 5
+
+# if value <= 3:
+#     value = 5
+# elif value == 3:
+#     value = 2
+
+# if value > 10:
+#     value = 10
+# else:
+#     value = 3
diff --git a/tests/functional/c/consider_using_min_max_builtin.txt b/tests/functional/c/consider_using_min_max_builtin.txt
new file mode 100644
index 0000000000..77be1d8ca3
--- /dev/null
+++ b/tests/functional/c/consider_using_min_max_builtin.txt
@@ -0,0 +1,11 @@
+consider-using-max-builtin:8::Consider using "value = max(value, 10)" instead of the unnecessary if block
+consider-using-min-builtin:11::Consider using "value = min(value, 10)" instead of the unnecessary if block
+consider-using-max-builtin:14::Consider using "value = max(value, 10)" instead of the unnecessary if block
+consider-using-min-builtin:17::Consider using "value = min(value, 10)" instead of the unnecessary if block
+consider-using-max-builtin:20::Consider using "value = max(value, value2)" instead of the unnecessary if block
+consider-using-min-builtin:23::Consider using "value = min(value, value2)" instead of the unnecessary if block
+consider-using-min-builtin:50::Consider using "A1 = min(A1, A2)" instead of the unnecessary if block
+consider-using-max-builtin:53::Consider using "A2 = max(A2, A1)" instead of the unnecessary if block
+consider-using-min-builtin:56::Consider using "A1 = min(A1, A2)" instead of the unnecessary if block
+consider-using-max-builtin:59::Consider using "A2 = max(A2, A1)" instead of the unnecessary if block
+

From 356c598ebc6c774d8fb2cec8eb3ca1c2a2023bdb Mon Sep 17 00:00:00 2001
From: Qwiddle <qwiddle@gmail.com>
Date: Wed, 14 Apr 2021 15:17:16 -0400
Subject: [PATCH 02/11] Added contributors

---
 CONTRIBUTORS.txt | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/CONTRIBUTORS.txt b/CONTRIBUTORS.txt
index 2160884db4..0252731b58 100644
--- a/CONTRIBUTORS.txt
+++ b/CONTRIBUTORS.txt
@@ -472,3 +472,7 @@ contributors:
 * Sebastian Müller: contributor
 
 * Ramiro Leal-Cavazos (ramiro050): Fixed bug preventing pylint from working with emacs tramp
+
+* manderj: contributor
+
+* qwiddle: contributor

From 700355bfa2e3b4360089aaf3383406d5cd2b9ed7 Mon Sep 17 00:00:00 2001
From: Qwiddle <qwiddle@gmail.com>
Date: Thu, 15 Apr 2021 00:05:49 -0400
Subject: [PATCH 03/11] Seems to pass consider_using_min_max_builtin.py test
 case

---
 .../refactoring/refactoring_checker.py        |  2 +-
 .../consider_using_min_max_builtin.py         | 97 +++++++++++++++++++
 .../consider_using_min_max_builtin.txt        | 11 +++
 .../c/consider_using_min_max_builtin.py       | 89 -----------------
 .../c/consider_using_min_max_builtin.txt      | 11 ---
 5 files changed, 109 insertions(+), 101 deletions(-)
 create mode 100644 tests/functional/c/consider/consider_using_min_max_builtin.py
 create mode 100644 tests/functional/c/consider/consider_using_min_max_builtin.txt
 delete mode 100644 tests/functional/c/consider_using_min_max_builtin.py
 delete mode 100644 tests/functional/c/consider_using_min_max_builtin.txt

diff --git a/pylint/checkers/refactoring/refactoring_checker.py b/pylint/checkers/refactoring/refactoring_checker.py
index f1b6f67a34..3e7b172659 100644
--- a/pylint/checkers/refactoring/refactoring_checker.py
+++ b/pylint/checkers/refactoring/refactoring_checker.py
@@ -288,6 +288,7 @@ class RefactoringChecker(checkers.BaseTokenChecker):
             'Consider using "%s" instead of unnecessary if block',
             "consider-using-max-builtin",
             "Using the max builtin improves readability",
+        ),
         "R1728": (
             "Consider using a generator instead '%s(%s)'",
             "consider-using-generator",
@@ -646,7 +647,6 @@ def _check_consider_using_min_max_builtin(self, node):
 
         # Check that the assignation is on the same variable.
         # TODO: retrieve full class name
-        #import ipdb; ipdb.set_trace()
         if hasattr(node.test.left, 'name'):
             left_operand = node.test.left.name
         elif hasattr(node.test.left, 'attrname'):
diff --git a/tests/functional/c/consider/consider_using_min_max_builtin.py b/tests/functional/c/consider/consider_using_min_max_builtin.py
new file mode 100644
index 0000000000..e12e4992ff
--- /dev/null
+++ b/tests/functional/c/consider/consider_using_min_max_builtin.py
@@ -0,0 +1,97 @@
+# pylint: disable=missing-docstring, invalid-name, too-few-public-methods, redefined-outer-name
+
+value = 10
+value2 = 0
+value3 = 3
+
+# Positive
+if value < 10:  # [consider-using-max-builtin]
+    value = 10
+
+if value >= 10:  # [consider-using-min-builtin]
+    value = 10
+
+if value <= 10:  # [consider-using-max-builtin]
+    value = 10
+
+if value > 10:  # [consider-using-min-builtin]
+    value = 10
+
+if value < value2:  # [consider-using-max-builtin]
+    value = value2
+
+if value > value2:  # [consider-using-min-builtin]
+    value = value2
+
+
+class A:
+    def __init__(self):
+        self.value = 13
+
+
+A1 = A()
+if A1.value > 10:  # [consider-using-min-builtin]
+    A1.value = 10
+
+
+class AA:
+    def __init__(self, value):
+        self.value = value
+
+    def __gt__(self, b):
+        return self.value > b
+
+    def __ge__(self, b):
+        return self.value >= b
+
+    def __lt__(self, b):
+        return self.value < b
+
+    def __le__(self, b):
+        return self.value <= b
+
+
+A1 = AA(0)
+A2 = AA(3)
+
+if A1 > A2:  # [consider-using-min-builtin]
+    A1 = A2
+
+if A2 < A1:  # [consider-using-max-builtin]
+    A2 = A1
+
+if A1 >= A2:  # [consider-using-min-builtin]
+    A1 = A2
+
+if A2 <= A1:  # [consider-using-max-builtin]
+    A2 = A1
+
+# Negative
+if value > 10:
+    value = 2
+
+if value > 10:
+    value = 2
+    value2 = 3
+
+if value > value2:
+    value = value3
+
+if value > 5:
+    value = value3
+
+if 2 < value <= 3:
+    value = 1
+
+if value <= 3:
+    value = 5
+
+if value <= 3:
+    value = 5
+elif value == 3:
+    value = 2
+
+if value > 10:
+    value = 10
+else:
+    value = 3
diff --git a/tests/functional/c/consider/consider_using_min_max_builtin.txt b/tests/functional/c/consider/consider_using_min_max_builtin.txt
new file mode 100644
index 0000000000..fd9d4cd68e
--- /dev/null
+++ b/tests/functional/c/consider/consider_using_min_max_builtin.txt
@@ -0,0 +1,11 @@
+consider-using-max-builtin:8:0::"Consider using ""value = max(value, 10)"" instead of unnecessary if block"
+consider-using-min-builtin:11:0::"Consider using ""value = min(value, 10)"" instead of unnecessary if block"
+consider-using-max-builtin:14:0::"Consider using ""value = max(value, 10)"" instead of unnecessary if block"
+consider-using-min-builtin:17:0::"Consider using ""value = min(value, 10)"" instead of unnecessary if block"
+consider-using-max-builtin:20:0::"Consider using ""value = max(value, value2)"" instead of unnecessary if block"
+consider-using-min-builtin:23:0::"Consider using ""value = min(value, value2)"" instead of unnecessary if block"
+consider-using-min-builtin:33:0::"Consider using ""value = min(value, 10)"" instead of unnecessary if block"
+consider-using-min-builtin:57:0::"Consider using ""A1 = min(A1, A2)"" instead of unnecessary if block"
+consider-using-max-builtin:60:0::"Consider using ""A2 = max(A2, A1)"" instead of unnecessary if block"
+consider-using-min-builtin:63:0::"Consider using ""A1 = min(A1, A2)"" instead of unnecessary if block"
+consider-using-max-builtin:66:0::"Consider using ""A2 = max(A2, A1)"" instead of unnecessary if block"
diff --git a/tests/functional/c/consider_using_min_max_builtin.py b/tests/functional/c/consider_using_min_max_builtin.py
deleted file mode 100644
index 8d573dde0c..0000000000
--- a/tests/functional/c/consider_using_min_max_builtin.py
+++ /dev/null
@@ -1,89 +0,0 @@
-# pylint: disable=missing-docstring, invalid-name, too-few-public-methods, redefined-outer-name
-
-# value = 10
-# value2 = 0
-# value3 = 3
-
-# # Positive
-# if value < 10: # [consider-using-max-builtin]
-#     value = 10
-
-# if value >= 10: # [consider-using-min-builtin]
-#     value = 10
-
-# if value <= 10: # [consider-using-max-builtin]
-#     value = 10
-
-# if value > 10: # [consider-using-min-builtin]
-#     value = 10
-
-# if value < value2: # [consider-using-max-builtin]
-#     value = value2
-
-# if value > value2: # [consider-using-min-builtin]
-#     value = value2
-
-class A:
-    def __init__(self):
-        self.value = 13
-
-A1 = A()
-if A1.value > 10: # [consider-using-min-builtin]
-    A1.value = 10
-
-# class AA:
-#     def __init__(self, value):
-#         self.value = value
-#     def __gt__(self, b):
-#         return self.value > b
-#     def __ge__(self, b):
-#         return self.value >= b
-#     def __lt__(self, b):
-#         return self.value < b
-#     def __le__(self, b):
-#         return self.value <= b
-
-# A1 = AA(0)
-# A2 = AA(3)
-
-# if A1 > A2: # [consider-using-min-builtin]
-#     A1 = A2
-
-# if A2 < A1: # [consider-using-max-builtin]
-#     A2 = A1
-
-# if A1 >= A2: # [consider-using-min-builtin]
-#     A1 = A2
-
-# if A2 <= A1: # [consider-using-max-builtin]
-#     A2 = A1
-
-# # Negative
-# if value > 10:
-#     value = 2
-
-# if value > 10:
-#    value = 2
-#    value2 = 3
-
-# if value > value2:
-#     value = value3
-
-# if value > 5:
-#     value = value3
-
-# if 2 < value <= 3:
-#     value = 1
-
-# if value <= 3:
-#     value = 5
-
-# if value <= 3:
-#     value = 5
-# elif value == 3:
-#     value = 2
-
-# if value > 10:
-#     value = 10
-# else:
-#     value = 3
diff --git a/tests/functional/c/consider_using_min_max_builtin.txt b/tests/functional/c/consider_using_min_max_builtin.txt
deleted file mode 100644
index 77be1d8ca3..0000000000
--- a/tests/functional/c/consider_using_min_max_builtin.txt
+++ /dev/null
@@ -1,11 +0,0 @@
-consider-using-max-builtin:8::Consider using "value = max(value, 10)" instead of the unnecessary if block
-consider-using-min-builtin:11::Consider using "value = min(value, 10)" instead of the unnecessary if block
-consider-using-max-builtin:14::Consider using "value = max(value, 10)" instead of the unnecessary if block
-consider-using-min-builtin:17::Consider using "value = min(value, 10)" instead of the unnecessary if block
-consider-using-max-builtin:20::Consider using "value = max(value, value2)" instead of the unnecessary if block
-consider-using-min-builtin:23::Consider using "value = min(value, value2)" instead of the unnecessary if block
-consider-using-min-builtin:50::Consider using "A1 = min(A1, A2)" instead of the unnecessary if block
-consider-using-max-builtin:53::Consider using "A2 = max(A2, A1)" instead of the unnecessary if block
-consider-using-min-builtin:56::Consider using "A1 = min(A1, A2)" instead of the unnecessary if block
-consider-using-max-builtin:59::Consider using "A2 = max(A2, A1)" instead of the unnecessary if block
-

From 3bb222f624629fcac24ec697039541a06db7eaa6 Mon Sep 17 00:00:00 2001
From: Qwiddle <qwiddle@gmail.com>
Date: Thu, 15 Apr 2021 00:21:59 -0400
Subject: [PATCH 04/11] Changed message numbers to not overwrite existing
 message types

---
 .../refactoring/refactoring_checker.py        | 20 +++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/pylint/checkers/refactoring/refactoring_checker.py b/pylint/checkers/refactoring/refactoring_checker.py
index 3e7b172659..ac2a0b74ff 100644
--- a/pylint/checkers/refactoring/refactoring_checker.py
+++ b/pylint/checkers/refactoring/refactoring_checker.py
@@ -279,16 +279,6 @@ class RefactoringChecker(checkers.BaseTokenChecker):
             "Emitted when calling the super() builtin with the current class "
             "and instance. On Python 3 these arguments are the default and they can be omitted.",
         ),
-        "R1726": (
-            'Consider using "%s" instead of unnecessary if block',
-            "consider-using-min-builtin",
-            "Using the min builtin improves readability",
-        ),
-        "R1727": (
-            'Consider using "%s" instead of unnecessary if block',
-            "consider-using-max-builtin",
-            "Using the max builtin improves readability",
-        ),
         "R1728": (
             "Consider using a generator instead '%s(%s)'",
             "consider-using-generator",
@@ -301,6 +291,16 @@ class RefactoringChecker(checkers.BaseTokenChecker):
             "Comprehension inside of 'any' or 'all' is unnecessary. "
             "A generator would be sufficient and faster.",
         ),
+        "R1730": (
+            'Consider using "%s" instead of unnecessary if block',
+            "consider-using-min-builtin",
+            "Using the min builtin improves readability",
+        ),
+        "R1731": (
+            'Consider using "%s" instead of unnecessary if block',
+            "consider-using-max-builtin",
+            "Using the max builtin improves readability",
+        ),
     }
     options = (
         (

From 38386c7441e0c95da99617827945ddc56c3360c7 Mon Sep 17 00:00:00 2001
From: Qwiddle <qwiddle@gmail.com>
Date: Thu, 15 Apr 2021 00:37:30 -0400
Subject: [PATCH 05/11] Removed TODO that I think was irrelevant

---
 pylint/checkers/refactoring/refactoring_checker.py | 1 -
 1 file changed, 1 deletion(-)

diff --git a/pylint/checkers/refactoring/refactoring_checker.py b/pylint/checkers/refactoring/refactoring_checker.py
index ac2a0b74ff..77c3e20fb0 100644
--- a/pylint/checkers/refactoring/refactoring_checker.py
+++ b/pylint/checkers/refactoring/refactoring_checker.py
@@ -646,7 +646,6 @@ def _check_consider_using_min_max_builtin(self, node):
             return
 
         # Check that the assignation is on the same variable.
-        # TODO: retrieve full class name
         if hasattr(node.test.left, 'name'):
             left_operand = node.test.left.name
         elif hasattr(node.test.left, 'attrname'):

From 30dce0a0461238aadaf93b444825ac6ba947d692 Mon Sep 17 00:00:00 2001
From: Qwiddle <qwiddle@gmail.com>
Date: Thu, 15 Apr 2021 00:58:35 -0400
Subject: [PATCH 06/11] Moved changelog entry to current version and added to
 what's new

---
 ChangeLog            | 8 ++++----
 doc/whatsnew/2.8.rst | 2 ++
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index d657326fa8..a776c1a400 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -35,6 +35,10 @@ Release date: Undefined
 
   Closes #2822, #4206, #4284
 
+* Add `consider-using-min-max-builtin` check for if statement which could be replaced by Python builtin min or max
+
+  Closes #3406
+
 
 What's New in Pylint 2.7.5?
 ===========================
@@ -408,10 +412,6 @@ What's New in Pylint 2.6.0?
 
 Release date: 2020-08-20
 
-* Add `consider-using-min-max-builtin` check for if statement which could be replaced by Python builtin min or max
-
-  Close #3406
-
 * Fix various scope-related bugs in ``undefined-variable`` checker
 
   Close #1082, #3434, #3461
diff --git a/doc/whatsnew/2.8.rst b/doc/whatsnew/2.8.rst
index 74beb7de43..11bce8c364 100644
--- a/doc/whatsnew/2.8.rst
+++ b/doc/whatsnew/2.8.rst
@@ -17,6 +17,8 @@ New checkers
 * Add new extension ``ConfusingConsecutiveElifChecker``. This optional checker emits a refactoring message (R5601 ``confusing-consecutive-elif``)
   if if/elif statements with different indentation levels follow directly one after the other.
 
+* Add ``consider-using-min-max-builtin`` check for if statement which could be replaced by Python builtin min or max.
+
 Other Changes
 =============
 

From 2384636bec0062da555de07c2f1a4d9b9131a472 Mon Sep 17 00:00:00 2001
From: Qwiddle <qwiddle@gmail.com>
Date: Thu, 15 Apr 2021 01:34:18 -0400
Subject: [PATCH 07/11] Setup pre-commit to comply

---
 ChangeLog                                     |  2 +-
 .../refactoring/refactoring_checker.py        | 31 ++++++++++++-------
 2 files changed, 20 insertions(+), 13 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index a776c1a400..a1dd0606ac 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -35,7 +35,7 @@ Release date: Undefined
 
   Closes #2822, #4206, #4284
 
-* Add `consider-using-min-max-builtin` check for if statement which could be replaced by Python builtin min or max
+* Add ``consider-using-min-max-builtin`` check for if statement which could be replaced by Python builtin min or max
 
   Closes #3406
 
diff --git a/pylint/checkers/refactoring/refactoring_checker.py b/pylint/checkers/refactoring/refactoring_checker.py
index 77c3e20fb0..0f4417e880 100644
--- a/pylint/checkers/refactoring/refactoring_checker.py
+++ b/pylint/checkers/refactoring/refactoring_checker.py
@@ -622,8 +622,7 @@ def visit_if(self, node):
         self._check_consider_using_min_max_builtin(node)
 
     def _check_consider_using_min_max_builtin(self, node):
-        """Check if the given if node can be refactored as an min/max python builtin.
-        """
+        """Check if the given if node can be refactored as an min/max python builtin."""
         if self._is_actual_elif(node) or node.orelse:
             # Not interested in if statements with multiple branches.
             return
@@ -633,11 +632,11 @@ def _check_consider_using_min_max_builtin(self, node):
 
         body = node.body[0]
         # Check if condition can be reduced.
-        if not hasattr(body, 'targets') or len(body.targets) != 1:
+        if not hasattr(body, "targets") or len(body.targets) != 1:
             return
 
         target = body.targets[0]
-        if not(
+        if not (
             isinstance(node.test, astroid.Compare)
             and not isinstance(target, astroid.Subscript)
             and not isinstance(node.test.left, astroid.Subscript)
@@ -646,16 +645,16 @@ def _check_consider_using_min_max_builtin(self, node):
             return
 
         # Check that the assignation is on the same variable.
-        if hasattr(node.test.left, 'name'):
+        if hasattr(node.test.left, "name"):
             left_operand = node.test.left.name
-        elif hasattr(node.test.left, 'attrname'):
+        elif hasattr(node.test.left, "attrname"):
             left_operand = node.test.left.attrname
         else:
             return
 
-        if hasattr(target, 'name'):
+        if hasattr(target, "name"):
             target_assignation = target.name
-        elif hasattr(target, 'attrname'):
+        elif hasattr(target, "attrname"):
             target_assignation = target.attrname
         else:
             return
@@ -685,11 +684,19 @@ def _check_consider_using_min_max_builtin(self, node):
             return
 
         if operator in ("<", "<="):
-            reduced_to = "{target} = max({target}, {item})".format(target=target_assignation, item=body_value)
-            self.add_message("consider-using-max-builtin", node=node, args=(reduced_to,))
+            reduced_to = "{target} = max({target}, {item})".format(
+                target=target_assignation, item=body_value
+            )
+            self.add_message(
+                "consider-using-max-builtin", node=node, args=(reduced_to,)
+            )
         elif operator in (">", ">="):
-            reduced_to = "{target} = min({target}, {item})".format(target=target_assignation, item=body_value)
-            self.add_message("consider-using-min-builtin", node=node, args=(reduced_to,))
+            reduced_to = "{target} = min({target}, {item})".format(
+                target=target_assignation, item=body_value
+            )
+            self.add_message(
+                "consider-using-min-builtin", node=node, args=(reduced_to,)
+            )
 
     @utils.check_messages("simplifiable-if-expression")
     def visit_ifexp(self, node):

From 47033b390b08b5d8ebdaecb34279cbe90dd0416b Mon Sep 17 00:00:00 2001
From: Qwiddle13 <32040075+Qwiddle13@users.noreply.github.com>
Date: Sat, 17 Apr 2021 12:24:36 -0400
Subject: [PATCH 08/11] Apply suggestions from code review

Co-authored-by: Pierre Sassoulas <pierre.sassoulas@gmail.com>
---
 pylint/checkers/refactoring/refactoring_checker.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/pylint/checkers/refactoring/refactoring_checker.py b/pylint/checkers/refactoring/refactoring_checker.py
index 0f4417e880..e556a5a5df 100644
--- a/pylint/checkers/refactoring/refactoring_checker.py
+++ b/pylint/checkers/refactoring/refactoring_checker.py
@@ -292,14 +292,14 @@ class RefactoringChecker(checkers.BaseTokenChecker):
             "A generator would be sufficient and faster.",
         ),
         "R1730": (
-            'Consider using "%s" instead of unnecessary if block',
+            "Consider using '%s' instead of unnecessary if block",
             "consider-using-min-builtin",
-            "Using the min builtin improves readability",
+            "Using the min builtin instead of a conditional improves readability and conciseness.",
         ),
         "R1731": (
             'Consider using "%s" instead of unnecessary if block',
             "consider-using-max-builtin",
-            "Using the max builtin improves readability",
+            "Using the max builtin instead of a conditional improves readability and conciseness.",
         ),
     }
     options = (

From e745b2729e8dac170347f213fa9923aad30d7847 Mon Sep 17 00:00:00 2001
From: Qwiddle13 <32040075+Qwiddle13@users.noreply.github.com>
Date: Sat, 17 Apr 2021 12:27:18 -0400
Subject: [PATCH 09/11] Added type hint to
 _check_consider_using_min_max_builtin

The broadest type should be astroid.If since _check_consider_using_min_max_builtin is only called by visit_if. This aligns with what I saw at runtime.
---
 pylint/checkers/refactoring/refactoring_checker.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/pylint/checkers/refactoring/refactoring_checker.py b/pylint/checkers/refactoring/refactoring_checker.py
index e556a5a5df..d5b01fd6df 100644
--- a/pylint/checkers/refactoring/refactoring_checker.py
+++ b/pylint/checkers/refactoring/refactoring_checker.py
@@ -621,7 +621,7 @@ def visit_if(self, node):
         self._check_consider_get(node)
         self._check_consider_using_min_max_builtin(node)
 
-    def _check_consider_using_min_max_builtin(self, node):
+    def _check_consider_using_min_max_builtin(self, node: astroid.If):
         """Check if the given if node can be refactored as an min/max python builtin."""
         if self._is_actual_elif(node) or node.orelse:
             # Not interested in if statements with multiple branches.

From 69bd6463f130a37d45caa60ac16ef6b4e8b629a6 Mon Sep 17 00:00:00 2001
From: Qwiddle13 <32040075+Qwiddle13@users.noreply.github.com>
Date: Sat, 17 Apr 2021 12:34:38 -0400
Subject: [PATCH 10/11] Added missed error message changes

---
 pylint/checkers/refactoring/refactoring_checker.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/pylint/checkers/refactoring/refactoring_checker.py b/pylint/checkers/refactoring/refactoring_checker.py
index d5b01fd6df..bab86b4bf7 100644
--- a/pylint/checkers/refactoring/refactoring_checker.py
+++ b/pylint/checkers/refactoring/refactoring_checker.py
@@ -297,7 +297,7 @@ class RefactoringChecker(checkers.BaseTokenChecker):
             "Using the min builtin instead of a conditional improves readability and conciseness.",
         ),
         "R1731": (
-            'Consider using "%s" instead of unnecessary if block',
+            "Consider using '%s' instead of unnecessary if block",
             "consider-using-max-builtin",
             "Using the max builtin instead of a conditional improves readability and conciseness.",
         ),

From 058d60951dcd54a086795818fa68599c10273593 Mon Sep 17 00:00:00 2001
From: Qwiddle13 <32040075+Qwiddle13@users.noreply.github.com>
Date: Sat, 17 Apr 2021 12:37:00 -0400
Subject: [PATCH 11/11] Changed expected test output to match error message
 update

---
 .../consider_using_min_max_builtin.txt        | 22 +++++++++----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/tests/functional/c/consider/consider_using_min_max_builtin.txt b/tests/functional/c/consider/consider_using_min_max_builtin.txt
index fd9d4cd68e..9ed5e0559d 100644
--- a/tests/functional/c/consider/consider_using_min_max_builtin.txt
+++ b/tests/functional/c/consider/consider_using_min_max_builtin.txt
@@ -1,11 +1,11 @@
-consider-using-max-builtin:8:0::"Consider using ""value = max(value, 10)"" instead of unnecessary if block"
-consider-using-min-builtin:11:0::"Consider using ""value = min(value, 10)"" instead of unnecessary if block"
-consider-using-max-builtin:14:0::"Consider using ""value = max(value, 10)"" instead of unnecessary if block"
-consider-using-min-builtin:17:0::"Consider using ""value = min(value, 10)"" instead of unnecessary if block"
-consider-using-max-builtin:20:0::"Consider using ""value = max(value, value2)"" instead of unnecessary if block"
-consider-using-min-builtin:23:0::"Consider using ""value = min(value, value2)"" instead of unnecessary if block"
-consider-using-min-builtin:33:0::"Consider using ""value = min(value, 10)"" instead of unnecessary if block"
-consider-using-min-builtin:57:0::"Consider using ""A1 = min(A1, A2)"" instead of unnecessary if block"
-consider-using-max-builtin:60:0::"Consider using ""A2 = max(A2, A1)"" instead of unnecessary if block"
-consider-using-min-builtin:63:0::"Consider using ""A1 = min(A1, A2)"" instead of unnecessary if block"
-consider-using-max-builtin:66:0::"Consider using ""A2 = max(A2, A1)"" instead of unnecessary if block"
+consider-using-max-builtin:8:0::"Consider using 'value = max(value, 10)' instead of unnecessary if block"
+consider-using-min-builtin:11:0::"Consider using 'value = min(value, 10)' instead of unnecessary if block"
+consider-using-max-builtin:14:0::"Consider using 'value = max(value, 10)' instead of unnecessary if block"
+consider-using-min-builtin:17:0::"Consider using 'value = min(value, 10)' instead of unnecessary if block"
+consider-using-max-builtin:20:0::"Consider using 'value = max(value, value2)' instead of unnecessary if block"
+consider-using-min-builtin:23:0::"Consider using 'value = min(value, value2)' instead of unnecessary if block"
+consider-using-min-builtin:33:0::"Consider using 'value = min(value, 10)' instead of unnecessary if block"
+consider-using-min-builtin:57:0::"Consider using 'A1 = min(A1, A2)' instead of unnecessary if block"
+consider-using-max-builtin:60:0::"Consider using 'A2 = max(A2, A1)' instead of unnecessary if block"
+consider-using-min-builtin:63:0::"Consider using 'A1 = min(A1, A2)' instead of unnecessary if block"
+consider-using-max-builtin:66:0::"Consider using 'A2 = max(A2, A1)' instead of unnecessary if block"