Browse Source

Python linting macroses

Init
93a5052a4192d230f52b4b27c41c9c77bf8d5880
alevitskii 6 months ago
parent
commit
f3fc9bb5ec
4 changed files with 224 additions and 79 deletions
  1. 46 8
      build/conf/python.conf
  2. 113 15
      build/plugins/_dart_fields.py
  3. 3 56
      build/plugins/pybuild.py
  4. 62 0
      build/plugins/ytest.py

+ 46 - 8
build/conf/python.conf

@@ -179,6 +179,24 @@ macro NO_YMAKE_PYTHON3() {
     SET(YMAKE_PYTHON3_PEERDIR)
 }
 
+FLAKE_EXTRA_PARAMS=
+FLAKE_MIGRATIONS_CONFIG=
+FLAKE_CONFIG_FILES=build/config/tests/flake8/flake8.conf $FLAKE_MIGRATIONS_CONFIG
+when ($DISABLE_FLAKE8_MIGRATIONS == "yes") {
+    FLAKE_EXTRA_PARAMS="DISABLE_FLAKE8_MIGRATIONS=yes"
+}
+otherwise {
+    FLAKE_MIGRATIONS_CONFIG=build/rules/flake8/migrations.yaml
+}
+
+when ($FLAKE8_FILE_PROCESSING_TIME == "") {
+    FLAKE8_FILE_PROCESSING_TIME=1.5
+}
+
+when ($BLACK_FILE_PROCESSING_TIME == "") {
+    BLACK_FILE_PROCESSING_TIME=1.5
+}
+
 # tag:python-specific tag:deprecated tag:internal
 module _PY2_PROGRAM: _BASE_PY_PROGRAM {
     SET(MODULE_LANG PY2)
@@ -186,6 +204,7 @@ module _PY2_PROGRAM: _BASE_PY_PROGRAM {
     # Looks like we cannot avoid copy-paste util ymake supports multiple inheritance
     # We need to attach coverage.extractor to every py_program target, except pytest targets
     ADD_YTEST($MODULE_PREFIX$REALPRJNAME coverage.extractor)
+    STYLE_PY2_FLAKE8()
 }
 
 # tag:python-specific tag:deprecated
@@ -247,25 +266,39 @@ multimodule PY3_PROGRAM {
 }
 
 # tag:python-specific tag:test
-STYLE_PYTHON_VALUE=no
-STYLE_PYTHON_PYPROJECT_VALUE=
 ### @usage: STYLE_PYTHON([pyproject])
 ###
 ### Check python3 sources for style issues using black.
+BLACK_CONFIG_FILES=
 macro STYLE_PYTHON(pyproject...) {
-    SET(STYLE_PYTHON_VALUE yes)
-    SET(STYLE_PYTHON_PYPROJECT_VALUE ${pyproject})
+    BLACK_CONFIG_FILES=$pyproject build/config/tests/py_style/config.toml
+    _ADD_PY_LINTER_CHECK(NAME black LINTER tools/black_linter/black_linter FILE_PROCESSING_TIME $BLACK_FILE_PROCESSING_TIME CONFIGS $BLACK_CONFIG_FILES)
 }
 
 # tag:python-specific tag:test
-STYLE_RUFF_VALUE=no
-RUFF_CONFIG_PATHS_FILE=${ARCADIA_ROOT}/build/config/tests/ruff/ruff_config_paths.json
 ### @usage: STYLE_RUFF()
 ###
 ### Check python3 sources for style issues using ruff.
+RUFF_CONFIG_PATHS_FILE=build/config/tests/ruff/ruff_config_paths.json
 macro STYLE_RUFF() {
-    SET(STYLE_RUFF_VALUE yes)
-    SET_APPEND(_MAKEFILE_INCLUDE_LIKE_DEPS ${RUFF_CONFIG_PATHS_FILE})
+    SET_APPEND(_MAKEFILE_INCLUDE_LIKE_DEPS ${ARCADIA_ROOT}/${RUFF_CONFIG_PATHS_FILE})
+    _ADD_PY_LINTER_CHECK(NAME ruff LINTER tools/ruff_linter/bin/ruff_linter GLOBAL_RESOURCES build/external_resources/ruff CONFIGS $RUFF_CONFIG_PATHS_FILE)
+}
+
+# tag:python-specific tag:test
+### @usage: STYLE_FLAKE8()
+###
+### Check python3 sources for style issues using flake8.
+macro STYLE_FLAKE8() {
+    _ADD_PY_LINTER_CHECK(NAME flake8 LINTER tools/flake8_linter/flake8_linter GLOBAL_RESOURCES build/external_resources/flake8_py3 FILE_PROCESSING_TIME $FLAKE8_FILE_PROCESSING_TIME CONFIGS $FLAKE_CONFIG_FILES EXTRA_PARAMS $FLAKE_EXTRA_PARAMS)
+}
+
+# tag:python-specific tag:test
+### @usage: STYLE_PY2_FLAKE8()
+###
+### Check python3 sources for style issues using flake8.
+macro STYLE_PY2_FLAKE8() {
+    _ADD_PY_LINTER_CHECK(NAME py2_flake8 LINTER tools/flake8_linter/flake8_linter GLOBAL_RESOURCES build/external_resources/flake8_py2 FILE_PROCESSING_TIME $FLAKE8_FILE_PROCESSING_TIME CONFIGS $FLAKE_CONFIG_FILES EXTRA_PARAMS $FLAKE_EXTRA_PARAMS)
 }
 
 # tag:python-specific tag:test
@@ -302,6 +335,7 @@ module PYTEST_BIN: _BASE_PYTEST {
     .DEFAULT_NAME_GENERATOR=FullPath
     .ARGS_PARSER=Base
     SETUP_PYTEST_BIN()
+    STYLE_PY2_FLAKE8()
 }
 
 # tag:python-specific tag:test
@@ -344,6 +378,7 @@ module PY3TEST_BIN: _BASE_PY3_PROGRAM {
     .ARGS_PARSER=Base
     SET(MODULE_LANG PY3)
     SETUP_PYTEST_BIN()
+    STYLE_FLAKE8()
     PEERDIR+=library/python/pytest
 }
 
@@ -565,6 +600,7 @@ module PY2_LIBRARY: _LIBRARY {
     }
     SET(MODULE_LANG PY2)
 
+    STYLE_PY2_FLAKE8()
     ADD_CLANG_TIDY()
     when ($TIDY_ENABLED == "yes") {
         _MAKEFILE_INCLUDE_LIKE_DEPS+=${ARCADIA_ROOT}/build/yandex_specific/config/clang_tidy/tidy_project_map.json
@@ -611,6 +647,7 @@ module PY3_LIBRARY: _LIBRARY {
     }
     SET(MODULE_LANG PY3)
 
+    STYLE_FLAKE8()
     ADD_CLANG_TIDY()
     when ($TIDY_ENABLED == "yes") {
         _MAKEFILE_INCLUDE_LIKE_DEPS+=${ARCADIA_ROOT}/build/yandex_specific/config/clang_tidy/tidy_project_map.json
@@ -749,6 +786,7 @@ module _BASE_PY3_PROGRAM: _BASE_PROGRAM {
 module PY3_PROGRAM_BIN: _BASE_PY3_PROGRAM {
     # Look's like we cannot avoid copy-paste util ymake supports multiple inheritance
     # We need to attach coverage.extractor to every py_program target, except pytest targets
+    STYLE_FLAKE8()
     ADD_YTEST($MODULE_PREFIX$REALPRJNAME coverage.extractor)
 }
 

+ 113 - 15
build/plugins/_dart_fields.py

@@ -22,6 +22,9 @@ CANON_OUTPUT_STORAGE = 'canondata_storage'
 KTLINT_CURRENT_EDITOR_CONFIG = "arcadia/build/platform/java/ktlint/.editorconfig"
 KTLINT_OLD_EDITOR_CONFIG = "arcadia/build/platform/java/ktlint_old/.editorconfig"
 
+ARCADIA_ROOT = '${ARCADIA_ROOT}/'
+SOURCE_ROOT_SHORT = '$S/'
+
 
 class DartValueError(ValueError):
     pass
@@ -256,6 +259,15 @@ def _get_ts_test_data_dirs(unit):
     )
 
 
+@_common.lazy
+def get_linter_configs(unit, config_paths):
+    rel_config_path = _common.rootrel_arc_src(config_paths, unit)
+    arc_config_path = unit.resolve_arc_path(rel_config_path)
+    abs_config_path = unit.resolve(arc_config_path)
+    with open(abs_config_path, 'r') as fd:
+        return list(json.load(fd).values())
+
+
 class AndroidApkTestActivity:
     KEY = 'ANDROID_APK_TEST_ACTIVITY'
 
@@ -360,10 +372,10 @@ class CustomDependencies:
 
     @classmethod
     def depends_with_linter(cls, unit, flat_args, spec_args):
-        deps = []
-        _, linter = flat_args
-        deps.append(os.path.dirname(linter))
-        deps += spec_args.get('DEPENDS', [])
+        linter = Linter.value(unit, flat_args, spec_args)[Linter.KEY]
+        deps = spec_args.get('DEPENDS', []) + [os.path.dirname(linter)]
+        for dep in deps:
+            unit.ondepends(dep)
         return {cls.KEY: " ".join(deps)}
 
     @classmethod
@@ -478,7 +490,7 @@ class JavaClasspathCmdType:
                     unit.path(), java_cp_arg_type
                 )
             )
-            raise DartValueError
+            raise DartValueError()
         return {cls.KEY: java_cp_arg_type}
 
 
@@ -528,9 +540,69 @@ class KtlintBinary:
         return {cls.KEY: value}
 
 
+class Linter:
+    KEY = 'LINTER'
+
+    @classmethod
+    def value(cls, unit, flat_args, spec_args):
+        return {cls.KEY: spec_args['LINTER'][0]}
+
+
+class LintConfigs:
+    KEY = 'LINT-CONFIGS'
+
+    @classmethod
+    def value(cls, unit, flat_args, spec_args):
+        resolved_configs = []
+        configs = spec_args.get('CONFIGS', [])
+        for cfg in configs:
+            filename = unit.resolve(SOURCE_ROOT_SHORT + cfg)
+            if not os.path.exists(filename):
+                message = 'Configuration file {} is not found'.format(filename)
+                raise DartValueError(message)
+            resolved_configs.append(cfg)
+            if os.path.splitext(filename)[-1] == '.json':
+                cfgs = get_linter_configs(unit, cfg)
+                for c in cfgs:
+                    filename = unit.resolve(SOURCE_ROOT_SHORT + c)
+                    if not os.path.exists(filename):
+                        message = 'Configuration file {} is not found'.format(filename)
+                        raise DartValueError(message)
+                    resolved_configs.append(c)
+        return {cls.KEY: serialize_list(resolved_configs)}
+
+
+class LintExtraParams:
+    KEY = 'LINT-EXTRA-PARAMS'
+
+    @classmethod
+    def from_macro_args(cls, unit, flat_args, spec_args):
+        extra_params = spec_args.get('EXTRA_PARAMS', [])
+        for arg in extra_params:
+            if '=' not in arg:
+                message = 'Wrong EXTRA_PARAMS value: "{}". Values must have format "name=value".'.format(arg)
+                raise DartValueError(message)
+        return {cls.KEY: serialize_list(extra_params)}
+
+
 class LintFileProcessingTime:
     KEY = 'LINT-FILE-PROCESSING-TIME'
 
+    @classmethod
+    def from_macro_args(cls, unit, flat_args, spec_args):
+        return {cls.KEY: spec_args.get('FILE_PROCESSING_TIME', [''])[0]}
+
+
+class LintName:
+    KEY = 'LINT-NAME'
+
+    @classmethod
+    def value(cls, unit, flat_args, spec_args):
+        lint_name = spec_args['NAME'][0]
+        if lint_name in ('flake8', 'py2_flake8') and (unit.get('DISABLE_FLAKE8') or 'no') == 'yes':
+            raise DartValueError('Flake8 linting is disabled by `DISABLE_FLAKE8`')
+        return {cls.KEY: lint_name}
+
 
 class ModuleLang:
     KEY = 'MODULE_LANG'
@@ -841,9 +913,8 @@ class TestData:
 
         props, error_mgs = extract_java_system_properties(unit, get_values_list(unit, 'SYSTEM_PROPERTIES_VALUE'))
         if error_mgs:
-            # TODO move error reporting out of field classes
             ymake.report_configure_error(error_mgs)
-            raise DartValueError
+            raise DartValueError()
         for prop in props:
             if prop['type'] == 'file':
                 test_data.append(prop['path'].replace('${ARCADIA_ROOT}', 'arcadia'))
@@ -949,7 +1020,8 @@ class TestedProjectName:
 
 class TestFiles:
     KEY = 'TEST-FILES'
-    # TODO remove FILES, see DEVTOOLS-7052
+    # TODO remove FILES, see DEVTOOLS-7052, currently it's required
+    # https://a.yandex-team.ru/arcadia/devtools/ya/test/dartfile/__init__.py?rev=r14292146#L10
     KEY2 = 'FILES'
 
     @classmethod
@@ -993,32 +1065,55 @@ class TestFiles:
     @classmethod
     def test_srcs(cls, unit, flat_args, spec_args):
         test_files = get_values_list(unit, 'TEST_SRCS_VALUE')
-        return {cls.KEY: serialize_list(test_files)}
+        value = serialize_list(test_files)
+        return {cls.KEY: value, cls.KEY2: value}
 
     @classmethod
     def ts_test_srcs(cls, unit, flat_args, spec_args):
         test_files = get_values_list(unit, "_TS_TEST_SRCS_VALUE")
         test_files = _resolve_module_files(unit, unit.get("MODDIR"), test_files)
-        return {cls.KEY: serialize_list(test_files)}
+        value = serialize_list(test_files)
+        return {cls.KEY: value, cls.KEY2: value}
 
     @classmethod
     def ts_input_files(cls, unit, flat_args, spec_args):
         typecheck_files = get_values_list(unit, "TS_INPUT_FILES")
         test_files = [_common.resolve_common_const(f) for f in typecheck_files]
-        return {cls.KEY: serialize_list(test_files)}
+        value = serialize_list(test_files)
+        return {cls.KEY: value, cls.KEY2: value}
 
     @classmethod
     def ts_lint_srcs(cls, unit, flat_args, spec_args):
         test_files = get_values_list(unit, "_TS_LINT_SRCS_VALUE")
         test_files = _resolve_module_files(unit, unit.get("MODDIR"), test_files)
-        return {cls.KEY: serialize_list(test_files)}
+        value = serialize_list(test_files)
+        return {cls.KEY: value, cls.KEY2: value}
 
     @classmethod
     def stylesheets(cls, unit, flat_args, spec_args):
         test_files = get_values_list(unit, "_TS_STYLELINT_FILES")
         test_files = _resolve_module_files(unit, unit.get("MODDIR"), test_files)
+        value = serialize_list(test_files)
+        return {cls.KEY: value, cls.KEY2: value}
 
-        return {cls.KEY: serialize_list(test_files)}
+    @classmethod
+    def py_linter_files(cls, unit, flat_args, spec_args):
+        files = unit.get('PY_LINTER_FILES')
+        if not files:
+            raise DartValueError()
+        files = json.loads(files)
+        test_files = []
+        for path in files:
+            if path.startswith(ARCADIA_ROOT):
+                test_files.append(path.replace(ARCADIA_ROOT, SOURCE_ROOT_SHORT, 1))
+            elif path.startswith(SOURCE_ROOT_SHORT):
+                test_files.append(path)
+        if not test_files:
+            lint_name = LintName.value(unit, flat_args, spec_args)[LintName.KEY]
+            message = 'No files to lint for {}'.format(lint_name)
+            raise DartValueError(message)
+        test_files = serialize_list(test_files)
+        return {cls.KEY: test_files, cls.KEY2: test_files}
 
 
 class TestEnv:
@@ -1097,6 +1192,10 @@ class TestName:
         test_name = os.path.basename(os.path.join(unit.path(), unit.filename()).replace(".pkg", ""))
         return {cls.KEY: os.path.splitext(test_name)[0]}
 
+    @classmethod
+    def name_from_macro_args(cls, unit, flat_args, spec_args):
+        return {cls.KEY: spec_args['NAME'][0]}
+
 
 class TestPartition:
     KEY = 'TEST_PARTITION'
@@ -1181,9 +1280,8 @@ class SystemProperties:
     def value(cls, unit, flat_args, spec_args):
         props, error_mgs = extract_java_system_properties(unit, get_values_list(unit, 'SYSTEM_PROPERTIES_VALUE'))
         if error_mgs:
-            # TODO move error reporting out of field classes
             ymake.report_configure_error(error_mgs)
-            raise DartValueError
+            raise DartValueError()
 
         props = base64.b64encode(six.ensure_binary(json.dumps(props)))
         return {cls.KEY: props}

+ 3 - 56
build/plugins/pybuild.py

@@ -183,62 +183,9 @@ def add_python_lint_checks(unit, py_ver, files):
     if files and no_lint_value not in ("none", "none_internal"):
         resolved_files = get_resolved_files()
         if resolved_files:
-            flake8_cfg = 'build/config/tests/flake8/flake8.conf'
-            migrations_cfg = 'build/rules/flake8/migrations.yaml'
-            resource = "build/external_resources/flake8_py{}".format(py_ver)
-            lint_name = "py2_flake8" if py_ver == 2 else "flake8"
-            params = [lint_name, "tools/flake8_linter/flake8_linter"]
-            params += ["FILES"] + resolved_files
-            params += ["GLOBAL_RESOURCES", resource]
-            params += [
-                "FILE_PROCESSING_TIME",
-                unit.get("FLAKE8_FILE_PROCESSING_TIME") or DEFAULT_FLAKE8_FILE_PROCESSING_TIME,
-            ]
-
-            extra_params = []
-            if unit.get("DISABLE_FLAKE8_MIGRATIONS") == "yes":
-                extra_params.append("DISABLE_FLAKE8_MIGRATIONS=yes")
-                config_files = [flake8_cfg, '']
-            else:
-                config_files = [flake8_cfg, migrations_cfg]
-            params += ["CONFIGS"] + config_files
-
-            if extra_params:
-                params += ["EXTRA_PARAMS"] + extra_params
-            unit.on_add_linter_check(params)
-
-    # ruff related stuff
-    if unit.get('STYLE_RUFF_VALUE') == 'yes':
-        if no_lint_value in ("none", "none_internal"):
-            ymake.report_configure_error(
-                'NO_LINT() and STYLE_RUFF() can\'t be enabled both at the same time',
-            )
-
-        resolved_files = get_resolved_files()
-        if resolved_files:
-            resource = "build/external_resources/ruff"
-            params = ["ruff", "tools/ruff_linter/bin/ruff_linter"]
-            params += ["FILES"] + resolved_files
-            params += ["GLOBAL_RESOURCES", resource]
-            configs = [
-                rootrel_arc_src(unit.get('RUFF_CONFIG_PATHS_FILE'), unit),
-                'build/config/tests/ruff/ruff.toml',
-            ] + get_ruff_configs(unit)
-            params += ['CONFIGS'] + configs
-            unit.on_add_linter_check(params)
-
-    if files and unit.get('STYLE_PYTHON_VALUE') == 'yes' and is_py3(unit):
-        resolved_files = get_resolved_files()
-        if resolved_files:
-            black_cfg = unit.get('STYLE_PYTHON_PYPROJECT_VALUE') or 'build/config/tests/py_style/config.toml'
-            params = ['black', 'tools/black_linter/black_linter']
-            params += ['FILES'] + resolved_files
-            params += ['CONFIGS', black_cfg]
-            params += [
-                "FILE_PROCESSING_TIME",
-                unit.get("BLACK_FILE_PROCESSING_TIME") or DEFAULT_BLACK_FILE_PROCESSING_TIME,
-            ]
-            unit.on_add_linter_check(params)
+            # repeated PY_SRCS, TEST_SCRS+PY_SRCS
+            collected = json.loads(unit.get('PY_LINTER_FILES')) if unit.get('PY_LINTER_FILES') else []
+            unit.set(['PY_LINTER_FILES', json.dumps(resolved_files + collected)])
 
 
 def is_py3(unit):

+ 62 - 0
build/plugins/ytest.py

@@ -947,6 +947,68 @@ def onsetup_run_python(unit):
         unit.ondepends('contrib/tools/python')
 
 
+@_common.lazy
+def get_linter_configs(unit, config_paths):
+    rel_config_path = _common.rootrel_arc_src(config_paths, unit)
+    arc_config_path = unit.resolve_arc_path(rel_config_path)
+    abs_config_path = unit.resolve(arc_config_path)
+    with open(abs_config_path, 'r') as fd:
+        return list(json.load(fd).values())
+
+
+@df.with_fields(
+    (
+        df.LintName.value,
+        df.TestFiles.py_linter_files,
+        df.LintConfigs.value,
+        df.LintExtraParams.from_macro_args,
+        df.TestName.name_from_macro_args,
+        df.TestedProjectName.unit_name,
+        df.SourceFolderPath.normalized,
+        df.TestEnv.value,
+        df.UseArcadiaPython.value,
+        df.LintFileProcessingTime.from_macro_args,
+        df.Linter.value,
+        df.CustomDependencies.depends_with_linter,
+    )
+)
+def on_add_py_linter_check(fields, unit, *args):
+    if unit.get("TIDY") == "yes":
+        return
+
+    no_lint_value = _common.get_no_lint_value(unit)
+    if no_lint_value in ("none", "none_internal"):
+        return
+
+    unlimited = -1
+    keywords = {
+        "NAME": 1,
+        "LINTER": 1,
+        "DEPENDS": unlimited,
+        "FILES": unlimited,
+        "CONFIGS": unlimited,
+        "GLOBAL_RESOURCES": unlimited,
+        "FILE_PROCESSING_TIME": 1,
+        "EXTRA_PARAMS": unlimited,
+    }
+    _, spec_args = _common.sort_by_keywords(keywords, args)
+
+    global_resources = spec_args.get('GLOBAL_RESOURCES', [])
+    for resource in global_resources:
+        unit.onpeerdir(resource)
+    try:
+        dart_record = create_dart_record(fields, unit, (), spec_args)
+    except df.DartValueError as e:
+        if msg := str(e):
+            unit.message(['WARN', msg])
+        return
+    dart_record[df.ScriptRelPath.KEY] = 'custom_lint'
+
+    data = dump_test(unit, dart_record)
+    if data:
+        unit.set_property(["DART_DATA", data])
+
+
 def on_add_linter_check(unit, *args):
     if unit.get("TIDY") == "yes":
         return