Linter integration (#1143)

Add flake8, autopep8 and mypy to our build pipeline and fix the errors from new linters

BUILD.bazel

@@ -347,15 +347,24 @@ test_suite(
 )
 
 test_suite(
-    name = "tests-vulkan-generator",
+    name = "lint",
     tests = [
-        # Linting
+        # __BEGIN_LINT
         "//vulkan_generator:lint",
+        "//vulkan_generator/codegen_utils:lint",
+        "//vulkan_generator/handle_remapper:lint",
         "//vulkan_generator/tests:lint",
         "//vulkan_generator/vulkan_parser:lint",
         "//vulkan_generator/vulkan_utils:lint",
+        # __END_LINT
+    ],
+)
 
-        # Tests
+test_suite(
+    name = "tests-vulkan-generator",
+    tests = [
+        # Run the linting as part of Vulkan generator tests
+        "//:lint",  # DO NOT REMOVE!
         "//vulkan_generator/tests:test_vulkan_parsing",
     ],
 )
@@ -448,13 +457,7 @@ test_suite(
         "//replay2/handle_remapper:handle_remapper_test",
         "//test/integration/replay:go_default_test",
         "//test/integration/service:go_default_test",
-        "//vulkan_generator:lint",
-        "//vulkan_generator/codegen_utils:lint",
-        "//vulkan_generator/handle_remapper:lint",
-        "//vulkan_generator/tests:lint",
         "//vulkan_generator/tests:test_vulkan_parsing",
-        "//vulkan_generator/vulkan_parser:lint",
-        "//vulkan_generator/vulkan_utils:lint",
         # __END_TESTS
     ],
 )

kokoro/linux-test/test.sh

@@ -66,6 +66,12 @@ function test {
         $@
 }
 
+ # If linting fails we do not need to wait entire AGI tests
+test lint
+
+# Test Vulkan Generator first as its much smaller and independent than other packages
+test tests-vulkan-generator
+
 # Running all the tests in one go leads to an out-of-memory error on Kokoro, hence the division in smaller test sets
 test tests-core
 test tests-gapis-api
@@ -74,4 +80,3 @@ test tests-gapis-other
 test tests-gapir
 test tests-gapil
 test tests-general
-test tests-vulkan-generator

kokoro/presubmit/build.sh

@@ -45,12 +45,16 @@ sudo apt-get install -y clang-format-6.0
 # Get recent Android build tools.
 echo y | $ANDROID_HOME/tools/bin/sdkmanager --install 'build-tools;30.0.3'
 
+# Python Format tool
+python3 -m pip install autopep8==1.6.0 --user
+
 # Setup environment.
 export ANDROID_NDK_HOME=/opt/android-ndk-r16b
 export BAZEL=$BUILD_ROOT/bazel/bin/bazel
 export BUILDIFIER=$BUILD_ROOT/tools/bin/buildifier
 export BUILDOZER=$BUILD_ROOT/tools/bin/buildozer
 export CLANG_FORMAT=clang-format-6.0
+export AUTOPEP8=~/.local/bin/autopep8
 
 cd $SRC
 

kokoro/presubmit/presubmit.sh

@@ -18,6 +18,7 @@ BAZEL=${BAZEL:-bazel}
 BUILDIFIER=${BUILDIFIER:-buildifier}
 BUILDOZER=${BUILDOZER:-buildozer}
 CLANG_FORMAT=${CLANG_FORMAT:-clang-format}
+AUTOPEP8=${AUTOPEP8:-autopep8}
 
 if test -t 1; then
   ncolors=$(tput colors)
@@ -85,12 +86,41 @@ function run_buildozer() {
 }
 
 function run_enumerate_tests() {
-  TARGETS="$($BAZEL query --deleted_packages=//gapii/fuchsia --output label 'kind(".*_test rule", //...)' | sort -t: -k1,1 | awk '{print "        \""$0"\","}')"
+  # Get all the test targets in all packages except gapii/fuschia
+  TARGETS=$($BAZEL query --deleted_packages=//gapii/fuchsia --output label 'kind(".*_test rule", //...)')
+
+  # Exclude linting targets from tests
+  TARGETS=$(echo "$TARGETS" | grep -v ":lint_")
+
+  # Sort the test targets alphabetically
+  TARGETS=$(echo "$TARGETS" | sort -t: -k1,1)
+
+  # Indent the test target names to match the BUILD.bazel file
+  TARGETS=$(echo "$TARGETS" | awk '{print "        \""$0"\","}')
+
   OUT=$(mktemp)
   cp BUILD.bazel $OUT
   cat $OUT | awk -v "targets=${TARGETS//$'\n'/\\n}" 'begin {a=0} /__END_TESTS/ {a=0} { if (a==0) print $0;} /__BEGIN_TESTS/ { a=1; print targets }' > BUILD.bazel
 }
 
+function run_enumerate_lints() {
+  # Get all the linting test targets in Vulkan generator
+  TARGETS=$($BAZEL query 'attr(generator_function, py_lint, //vulkan_generator/...)')
+
+  # Exclude the sub targets to prevent repetation
+  TARGETS=$(echo "$TARGETS" | grep -v "_flake8" | grep -v "_mypy" | grep -v "_pylint")
+
+  # Sort the linting targets alphabetically
+  TARGETS=$(echo "$TARGETS" | sort -t: -k1,1)
+
+  # Indent the linting target names to match the BUILD.bazel file
+  TARGETS=$(echo "$TARGETS" | awk '{print "        \""$0"\","}')
+
+  OUT=$(mktemp)
+  cp BUILD.bazel $OUT
+  cat $OUT | awk -v "targets=${TARGETS//$'\n'/\\n}" 'begin {a=0} /__END_LINT/ {a=0} { if (a==0) print $0;} /__BEGIN_LINT/ { a=1; print targets }' > BUILD.bazel
+}
+
 function run_gazelle() {
   echo # TODO: figure out a way to make bazel not print anything.
   $BAZEL run gazelle
@@ -102,15 +132,16 @@ function run_gofmt() {
   find . -name "*.go" | xargs $GOFMT -w
 }
 
+function run_autopep8() {
+  $AUTOPEP8 --global-config=tools/build/python/pep8 -r --in-place vulkan_generator
+}
+
 # Ensure we are clean to start out with.
 check "git workspace must be clean" true
 
 # Check copyright headers
 check copyright-headers run_copyright_headers
 
-# Check clang-format.
-check clang-format run_clang_format
-
 # Check buildifier.
 check buildifier run_buildifier
 
@@ -120,12 +151,21 @@ check "buildozer fix" run_buildozer
 # Check that the //:tests target contains all tests.
 check "//:tests contains all tests" run_enumerate_tests
 
+# Check that "//:lint" target contains all lints
+check "//:lint contains all lints" run_enumerate_lints
+
 # Check gazelle.
 check "gazelle" run_gazelle
 
 # Check gofmt. This needs to be done AFTER Gazelle, such that Bazel has
 # installed its Go SDK (as a dependency to run Gazelle).
 check gofmt run_gofmt
 
+# Check python Formatter(autopep8)
+check autopep8 run_autopep8
+
+# Check clang-format.
+check clang-format run_clang_format
+
 echo
 echo "${green}All check completed successfully.$normal"

tools/build/python/BUILD.bazel

@@ -13,7 +13,11 @@
 # limitations under the License.
 
 exports_files([
-    "py_lint.py",
+    "lint_flake8.py",
+    "lint_mypy.py",
+    "lint_pylint.py",
+    "flake8",
+    "mypy.ini",
     "py_test.py",
     "pylintrc",
     "requirements.txt",

tools/build/python/flake8

@@ -0,0 +1,17 @@
+# Copyright (C) 2022 Google Inc.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+
+[flake8]
+ignore=E501 # Max line length is already checked by pylint

tools/build/python/lint_flake8.py

@@ -0,0 +1,20 @@
+# Copyright (C) 2022 Google Inc.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+import sys
+from flake8.main import cli as flake8_cli
+
+
+if __name__ == "__main__":
+    flake8_cli.main(sys.argv[1:])

tools/build/python/lint_mypy.py

@@ -0,0 +1,20 @@
+# Copyright (C) 2022 Google Inc.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+import sys
+from mypy import main as mypy_main
+
+
+if __name__ == "__main__":
+    mypy_main.main(None, sys.stdout, sys.stderr, sys.argv[1:], clean_exit=True)

tools/build/python/py_lint.py → tools/build/python/lint_pylint.py

@@ -15,5 +15,6 @@
 import sys
 import pylint
 
+
 if __name__ == "__main__":
-    sys.exit(pylint.run_pylint(sys.argv[1:]))
+    pylint.run_pylint(sys.argv[1:])

tools/build/python/mypy.ini

@@ -0,0 +1,23 @@
+# Copyright (C) 2022 Google Inc.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+
+[mypy]
+show_column_numbers=True
+pretty=False
+follow_imports=silent
+
+# Unless you are 110% sure, *NEVER* change this option
+# It will break both the type system and import discovery of mypy
+ignore_missing_imports=False

tools/build/python/pep8

@@ -0,0 +1,16 @@
+# Copyright (C) 2022 Google Inc.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+[pep8]
+max-line-length = 120

tools/build/python/python.bzl

@@ -1,13 +1,13 @@
 load("@rules_python//python:defs.bzl", _py_test = "py_test")
 load("@pip//:requirements.bzl", "requirement")
 
-def py_lint(name, srcs, deps = [], args = [], **kwargs):
+def _lint_pylint(name, srcs, deps = [], args = [], **kwargs):
     _py_test(
         name = name,
         srcs = [
-            "//tools/build/python:py_lint.py",
+            "//tools/build/python:lint_pylint.py",
         ] + srcs,
-        main = "//tools/build/python:py_lint.py",
+        main = "//tools/build/python:lint_pylint.py",
         args = [
             "--rcfile=$(location //tools/build/python:pylintrc)"
         ] + args + ["$(location :%s)" % x for x in srcs],
@@ -20,6 +20,58 @@ def py_lint(name, srcs, deps = [], args = [], **kwargs):
         **kwargs
     )
 
+def _lint_mypy(name, srcs, deps = [], args = [], **kwargs):
+    _py_test(
+        name = name,
+        srcs = [
+            "//tools/build/python:lint_mypy.py",
+        ] + srcs,
+        main = "//tools/build/python:lint_mypy.py",
+        args = [
+            "--config-file=$(location //tools/build/python:mypy.ini)"
+        ] + args + ["$(location :%s)" % x for x in srcs],
+        python_version = "PY3",
+        srcs_version = "PY3",
+        data = [ "//tools/build/python:mypy.ini" ],
+        deps = deps + [
+            requirement("mypy"),
+        ],
+        **kwargs
+    )
+
+def _lint_flake8(name, srcs, deps = [], args = [], **kwargs):
+    _py_test(
+        name = name,
+        srcs = [
+            "//tools/build/python:lint_flake8.py",
+        ] + srcs,
+        main = "//tools/build/python:lint_flake8.py",
+        args = [
+            "--config=$(location //tools/build/python:flake8)"
+        ] + args + ["$(location :%s)" % x for x in srcs],
+        python_version = "PY3",
+        srcs_version = "PY3",
+        data = [ "//tools/build/python:flake8" ],
+        deps = deps + [
+            requirement("flake8"),
+        ],
+        **kwargs
+    )
+
+def py_lint(name, srcs, deps = [], args = [], **kwargs):
+    _lint_mypy(name + "_mypy", srcs, deps, args, **kwargs)
+    _lint_pylint(name + "_pylint", srcs, deps, args, **kwargs)
+    _lint_flake8(name + "_flake8", srcs, deps, args, **kwargs)
+
+    native.test_suite(
+        name=name,
+        tests=[
+            name + "_flake8",
+            name + "_mypy",
+            name + "_pylint",
+        ]
+    )
+
 def py_test(name, srcs, deps = [], args = [], **kwargs):
     _py_test(
         name = name,

tools/build/python/requirements.txt

@@ -1,2 +1,4 @@
 pytest==7.1.2
 pylint==2.13.8
+mypy==0.950
+flake8==4.0.1

vulkan_generator/BUILD.bazel

@@ -47,4 +47,7 @@ py_binary(
 py_lint(
     name = "lint",
     srcs = glob(["*.py"]),
+    deps = [
+        "//vulkan_generator",
+    ],
 )

vulkan_generator/__init__.py

@@ -0,0 +1,15 @@
+# Copyright (C) 2022 Google Inc.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""Top level package for Vulkan Generator"""