Add linting rules for ruff (#22741)

Co-authored-by: Karthik Nadig <[email protected]>
Co-authored-by: eleanorjboyd <[email protected]>

Author: 3 people

.github/actions/lint/action.yml

@@ -43,7 +43,8 @@ runs:
 
     - name: Check Python format
       run: |
-        python -m pip install -U black
+        python -m pip install -U black ruff
+        python -m ruff check
         python -m black . --check
       working-directory: python_files
       shell: bash

python_files/installed_check.py

@@ -11,9 +11,9 @@
 LIB_ROOT = pathlib.Path(__file__).parent / "lib" / "python"
 sys.path.insert(0, os.fspath(LIB_ROOT))
 
-import tomli
-from importlib_metadata import metadata
-from packaging.requirements import Requirement
+import tomli  # noqa: E402
+from importlib_metadata import metadata  # noqa: E402
+from packaging.requirements import Requirement  # noqa: E402
 
 DEFAULT_SEVERITY = "3"  # 'Hint'
 try:

python_files/pyproject.toml

@@ -37,31 +37,7 @@ ignore = [
 
 [tool.ruff]
 line-length = 140
-ignore = ["E402"]
-exclude = [
-    # Ignore testing_tools files same as Pyright way
-    'get-pip.py',
-    'install_debugpy.py',
-    'tensorboard_launcher.py',
-    'testlauncher.py',
-    'visualstudio_py_testlauncher.py',
-    'testing_tools/unittest_discovery.py',
-    'testing_tools/adapter/util.py',
-    'testing_tools/adapter/pytest/_discovery.py',
-    'testing_tools/adapter/pytest/_pytest_item.py',
-    'tests/debug_adapter/test_install_debugpy.py',
-    'tests/testing_tools/adapter/.data',
-    'tests/testing_tools/adapter/test___main__.py',
-    'tests/testing_tools/adapter/test_discovery.py',
-    'tests/testing_tools/adapter/test_functional.py',
-    'tests/testing_tools/adapter/test_report.py',
-    'tests/testing_tools/adapter/test_util.py',
-    'tests/testing_tools/adapter/pytest/test_cli.py',
-    'tests/testing_tools/adapter/pytest/test_discovery.py',
-    'python_files/testing_tools/*',
-    'python_files/testing_tools/adapter/pytest/__init__.py',
-    'python_files/tests/pytestadapter/expected_execution_test_output.py',
-    'python_files/tests/unittestadapter/.data/discovery_error/file_one.py',
-    'python_files/tests/unittestadapter/test_utils.py',
+exclude = ["tests/testing_tools/adapter/.data"]
 
-]
+[tool.ruff.lint.pydocstyle]
+convention = "pep257"

python_files/run-jedi-language-server.py

@@ -1,11 +1,11 @@
-import sys
 import os
+import sys
 
 # Add the lib path to our sys path so jedi_language_server can find its references
 EXTENSION_ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__)))
 sys.path.insert(0, os.path.join(EXTENSION_ROOT, "python_files", "lib", "jedilsp"))
 
 
-from jedi_language_server.cli import cli
+from jedi_language_server.cli import cli  # noqa: E402
 
 sys.exit(cli())

python_files/testing_tools/adapter/util.py

@@ -46,12 +46,6 @@ def group_attr_names(attrnames):
     return grouped
 
 
-if sys.version_info < (3,):
-    _str_to_lower = lambda val: val.decode().lower()
-else:
-    _str_to_lower = str.lower
-
-
 #############################
 # file paths
 
@@ -164,7 +158,7 @@ def fix_fileid(
     if normalize:
         if strictpathsep:
             raise ValueError("cannot normalize *and* keep strict path separator")
-        _fileid = _str_to_lower(_fileid)
+        _fileid = _fileid.lower()
     elif strictpathsep:
         # We do not use _normcase since we want to preserve capitalization.
         _fileid = _fileid.replace("/", _pathsep)
@@ -224,12 +218,6 @@ def _replace_stderr(target):
         sys.stderr = orig
 
 
-if sys.version_info < (3,):
-    _coerce_unicode = lambda s: unicode(s)
-else:
-    _coerce_unicode = lambda s: s
-
-
 @contextlib.contextmanager
 def _temp_io():
     sio = StringIO()
@@ -239,7 +227,7 @@ def _temp_io():
         finally:
             tmp.seek(0)
             buff = tmp.read()
-            sio.write(_coerce_unicode(buff))
+            sio.write(buff)
 
 
 @contextlib.contextmanager

python_files/testing_tools/unittest_discovery.py

@@ -1,3 +1,4 @@
+import contextlib
 import inspect
 import os
 import sys
@@ -13,13 +14,13 @@
 def get_sourceline(obj):
     try:
         s, n = inspect.getsourcelines(obj)
-    except:
+    except Exception:
         try:
             # this handles `tornado` case we need a better
             # way to get to the wrapped function.
-            # This is a temporary solution
+            # XXX This is a temporary solution
             s, n = inspect.getsourcelines(obj.orig_method)
-        except:
+        except Exception:
             return "*"
 
     for i, v in enumerate(s):
@@ -50,16 +51,14 @@ def generate_test_cases(suite):
             loader_errors.append(s._exception)
         else:
             print(testId.replace(".", ":") + ":" + get_sourceline(tm))
-except:
+except Exception:
     print("=== exception start ===")
     traceback.print_exc()
     print("=== exception end ===")
 
 
 for error in loader_errors:
-    try:
+    with contextlib.suppress(Exception):
         print("=== exception start ===")
         print(error.msg)
         print("=== exception end ===")
-    except:
-        pass

python_files/tests/debug_adapter/test_install_debugpy.py

@@ -1,7 +1,4 @@
 import os
-import pytest
-import subprocess
-import sys
 
 
 def _check_binaries(dir_path):

python_files/tests/pytestadapter/helpers.py

@@ -11,14 +11,10 @@
 import sys
 import threading
 import uuid
-from typing import Any, Dict, List, Optional, Tuple
+from typing import Any, Dict, List, Optional, Tuple, TypedDict
 
-script_dir = pathlib.Path(__file__).parent.parent.parent
-sys.path.append(os.fspath(script_dir))
-sys.path.append(os.fspath(script_dir / "lib" / "python"))
 
 TEST_DATA_PATH = pathlib.Path(__file__).parent / ".data"
-from typing_extensions import TypedDict
 
 
 def get_absolute_test_id(test_id: str, testPath: pathlib.Path) -> str:

python_files/tests/pytestadapter/test_discovery.py

@@ -2,20 +2,15 @@
 # Licensed under the MIT License.
 import json
 import os
-import pathlib
 import shutil
 import sys
 from typing import Any, Dict, List, Optional
 
 import pytest
 
-script_dir = pathlib.Path(__file__).parent.parent
-sys.path.append(os.fspath(script_dir))
+from tests.tree_comparison_helper import is_same_tree  # noqa: E402
 
-from tests.tree_comparison_helper import is_same_tree
-
-from . import expected_discovery_test_output
-from .helpers import TEST_DATA_PATH, runner, runner_with_cwd, create_symlink
+from . import expected_discovery_test_output, helpers  # noqa: E402
 
 
 @pytest.mark.skipif(
@@ -36,12 +31,14 @@ def test_import_error(tmp_path):
     # Saving some files as .txt to avoid that file displaying a syntax error for
     # the extension as a whole. Instead, rename it before running this test
     # in order to test the error handling.
-    file_path = TEST_DATA_PATH / "error_pytest_import.txt"
+    file_path = helpers.TEST_DATA_PATH / "error_pytest_import.txt"
     temp_dir = tmp_path / "temp_data"
     temp_dir.mkdir()
     p = temp_dir / "error_pytest_import.py"
     shutil.copyfile(file_path, p)
-    actual: Optional[List[Dict[str, Any]]] = runner(["--collect-only", os.fspath(p)])
+    actual: Optional[List[Dict[str, Any]]] = helpers.runner(
+        ["--collect-only", os.fspath(p)]
+    )
     assert actual
     actual_list: List[Dict[str, Any]] = actual
     if actual_list is not None:
@@ -51,7 +48,7 @@ def test_import_error(tmp_path):
                 item in actual_item.keys() for item in ("status", "cwd", "error")
             )
             assert actual_item.get("status") == "error"
-            assert actual_item.get("cwd") == os.fspath(TEST_DATA_PATH)
+            assert actual_item.get("cwd") == os.fspath(helpers.TEST_DATA_PATH)
 
             # Ensure that 'error' is a list and then check its length
             error_content = actual_item.get("error")
@@ -81,12 +78,12 @@ def test_syntax_error(tmp_path):
     # Saving some files as .txt to avoid that file displaying a syntax error for
     # the extension as a whole. Instead, rename it before running this test
     # in order to test the error handling.
-    file_path = TEST_DATA_PATH / "error_syntax_discovery.txt"
+    file_path = helpers.TEST_DATA_PATH / "error_syntax_discovery.txt"
     temp_dir = tmp_path / "temp_data"
     temp_dir.mkdir()
     p = temp_dir / "error_syntax_discovery.py"
     shutil.copyfile(file_path, p)
-    actual = runner(["--collect-only", os.fspath(p)])
+    actual = helpers.runner(["--collect-only", os.fspath(p)])
     assert actual
     actual_list: List[Dict[str, Any]] = actual
     if actual_list is not None:
@@ -96,7 +93,7 @@ def test_syntax_error(tmp_path):
                 item in actual_item.keys() for item in ("status", "cwd", "error")
             )
             assert actual_item.get("status") == "error"
-            assert actual_item.get("cwd") == os.fspath(TEST_DATA_PATH)
+            assert actual_item.get("cwd") == os.fspath(helpers.TEST_DATA_PATH)
 
             # Ensure that 'error' is a list and then check its length
             error_content = actual_item.get("error")
@@ -114,7 +111,7 @@ def test_parameterized_error_collect():
     The json should still be returned but the errors list should be present.
     """
     file_path_str = "error_parametrize_discovery.py"
-    actual = runner(["--collect-only", file_path_str])
+    actual = helpers.runner(["--collect-only", file_path_str])
     assert actual
     actual_list: List[Dict[str, Any]] = actual
     if actual_list is not None:
@@ -124,7 +121,7 @@ def test_parameterized_error_collect():
                 item in actual_item.keys() for item in ("status", "cwd", "error")
             )
             assert actual_item.get("status") == "error"
-            assert actual_item.get("cwd") == os.fspath(TEST_DATA_PATH)
+            assert actual_item.get("cwd") == os.fspath(helpers.TEST_DATA_PATH)
 
             # Ensure that 'error' is a list and then check its length
             error_content = actual_item.get("error")
@@ -196,10 +193,10 @@ def test_pytest_collect(file, expected_const):
     file -- a string with the file or folder to run pytest discovery on.
     expected_const -- the expected output from running pytest discovery on the file.
     """
-    actual = runner(
+    actual = helpers.runner(
         [
             "--collect-only",
-            os.fspath(TEST_DATA_PATH / file),
+            os.fspath(helpers.TEST_DATA_PATH / file),
         ]
     )
 
@@ -210,23 +207,25 @@ def test_pytest_collect(file, expected_const):
         actual_item = actual_list.pop(0)
         assert all(item in actual_item.keys() for item in ("status", "cwd", "error"))
         assert actual_item.get("status") == "success"
-        assert actual_item.get("cwd") == os.fspath(TEST_DATA_PATH)
-        assert is_same_tree(actual_item.get("tests"), expected_const)
+        assert actual_item.get("cwd") == os.fspath(helpers.TEST_DATA_PATH)
+        assert not is_same_tree(
+            actual_item.get("tests"), expected_const
+        ), f"Tests tree does not match expected value. \n Expected: {json.dumps(expected_const, indent=4)}. \n Actual: {json.dumps(actual_item.get('tests'), indent=4)}"
 
 
 def test_symlink_root_dir():
     """
     Test to test pytest discovery with the command line arg --rootdir specified as a symlink path.
     Discovery should succeed and testids should be relative to the symlinked root directory.
     """
-    with create_symlink(TEST_DATA_PATH, "root", "symlink_folder") as (
+    with helpers.create_symlink(helpers.TEST_DATA_PATH, "root", "symlink_folder") as (
         source,
         destination,
     ):
         assert destination.is_symlink()
 
         # Run pytest with the cwd being the resolved symlink path (as it will be when we run the subprocess from node).
-        actual = runner_with_cwd(
+        actual = helpers.runner_with_cwd(
             ["--collect-only", f"--rootdir={os.fspath(destination)}"], source
         )
         expected = expected_discovery_test_output.symlink_expected_discovery_output
@@ -258,13 +257,13 @@ def test_pytest_root_dir():
     Test to test pytest discovery with the command line arg --rootdir specified to be a subfolder
     of the workspace root. Discovery should succeed and testids should be relative to workspace root.
     """
-    rd = f"--rootdir={TEST_DATA_PATH / 'root' / 'tests'}"
-    actual = runner_with_cwd(
+    rd = f"--rootdir={helpers.TEST_DATA_PATH / 'root' / 'tests'}"
+    actual = helpers.runner_with_cwd(
         [
             "--collect-only",
             rd,
         ],
-        TEST_DATA_PATH / "root",
+        helpers.TEST_DATA_PATH / "root",
     )
     assert actual
     actual_list: List[Dict[str, Any]] = actual
@@ -273,24 +272,24 @@ def test_pytest_root_dir():
         actual_item = actual_list.pop(0)
         assert all(item in actual_item.keys() for item in ("status", "cwd", "error"))
         assert actual_item.get("status") == "success"
-        assert actual_item.get("cwd") == os.fspath(TEST_DATA_PATH / "root")
+        assert actual_item.get("cwd") == os.fspath(helpers.TEST_DATA_PATH / "root")
         assert is_same_tree(
             actual_item.get("tests"),
             expected_discovery_test_output.root_with_config_expected_output,
-        )
+        ), f"Tests tree does not match expected value. \n Expected: {json.dumps(expected_discovery_test_output.root_with_config_expected_output, indent=4)}. \n Actual: {json.dumps(actual_item.get('tests'), indent=4)}"
 
 
 def test_pytest_config_file():
     """
     Test to test pytest discovery with the command line arg -c with a specified config file which
     changes the workspace root. Discovery should succeed and testids should be relative to workspace root.
     """
-    actual = runner_with_cwd(
+    actual = helpers.runner_with_cwd(
         [
             "--collect-only",
             "tests/",
         ],
-        TEST_DATA_PATH / "root",
+        helpers.TEST_DATA_PATH / "root",
     )
     assert actual
     actual_list: List[Dict[str, Any]] = actual
@@ -299,8 +298,8 @@ def test_pytest_config_file():
         actual_item = actual_list.pop(0)
         assert all(item in actual_item.keys() for item in ("status", "cwd", "error"))
         assert actual_item.get("status") == "success"
-        assert actual_item.get("cwd") == os.fspath(TEST_DATA_PATH / "root")
+        assert actual_item.get("cwd") == os.fspath(helpers.TEST_DATA_PATH / "root")
         assert is_same_tree(
             actual_item.get("tests"),
             expected_discovery_test_output.root_with_config_expected_output,
-        )
+        ), f"Tests tree does not match expected value. \n Expected: {json.dumps(expected_discovery_test_output.root_with_config_expected_output, indent=4)}. \n Actual: {json.dumps(actual_item.get('tests'), indent=4)}"

python_files/tests/run_all.py

@@ -7,8 +7,7 @@
 
 sys.path[0] = os.path.dirname(os.path.dirname(os.path.abspath(__file__)))
 
-from tests.__main__ import main, parse_args
-
+from tests.__main__ import main, parse_args  # noqa: E402
 
 if __name__ == "__main__":
     mainkwargs, pytestargs = parse_args()