Only run clang-tidy warning checks reported as errors

[jmagman]

lint.sh was running clang-tidy but only failing on the warnings explicitly treated as errors. That means it was running more checks than it was reporting, but not giving any additional information.

Treat all clang-tidy warnings as errors. This will also prevent checks being added that aren't really being reported.

Remove performance-move-const-arg and performance-unnecessary-value-param checks because they produce errors that were never surfaced and would need to be fixed before these checks are re-added.

It looks like this improved Mac Host clang-tidy time, but the rest are about the same.

• Linux Android clang-tidy
• Linux Host clang-tidy
• Mac iOS clang-tidy
• Mac Host clang-tidy

Pre-launch Checklist

• I read the Contributor Guide and followed the process outlined there for submitting PRs.
• I read the Tree Hygiene wiki page, which explains my responsibilities.
• I read and followed the Flutter Style Guide and the C++, Objective-C, Java style guides.
• I listed at least one issue that this PR fixes in the description above.
• I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt. See testing the engine for instructions on
  writing and running engine tests.
• I updated/added relevant documentation (doc comments with ///).
• I signed the CLA.
• All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

[jmagman]

That didn't work as expected, --warnings-as-errors is ignoring the disabled checks even though it shouldn't:

FlutterBinaryMessengerRelay.mm:42:5: error: nil returned from a method that is expected to return a non-null value [clang-analyzer-nullability.NullReturnedFromNonnull,-warnings-as-errors]

https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket/8799827550243419073/+/u/test:_lint_ios_debug_sim/stdout

Checks: "bugprone-use-after-move,\
...
-clang-analyzer-nullability.NullReturnedFromNonnull,\

  --warnings-as-errors=<string>  -
                                   Upgrades warnings to errors. Same format as
                                   '-checks'.
                                   This option's value is appended to the value of
                                   the 'WarningsAsErrors' option in .clang-tidy
                                   file, if any.

Edit: never mind there was a stray quote and so a parse error.

[gaaclarke]

Landing this as-is will break https://github.com/flutter/engine/pull/36910/files since the checks will be specified as errors for mac, but not be turned on as checks. Clang-tidy isn't smart about that. We need to make our clang-tidy app account for that or keep the checks on in the .clang-tidy file.

I think we should keep it on as a check since it helps out in limited cases. If we still want to remove it I can update the clang-tidy tool first. I don't want to put more work on your plate. We just have to make a decision @zanderso @jmagman.

[chinmaygarde]

From Triage: We were wondering if this can be resurrected now. Let's close it if not. In the meantime, I am marking this as a WIP.

[gaaclarke]

All of the checks that were removed here that have clang-tidy fix implementations can probably be turned on as errors now since I've ran clang-tidy fix to land the performance lint checks and they got cleaned up. For example the removal of c style casts should be complete now.

[jmagman]

For example the removal of c style casts should be complete now.

@gaaclarke I still see three on Linux host

❌ Failures for clang-tidy on /b/s/w/ir/cache/builder/src/flutter/flow/layers/backdrop_filter_layer.cc:
/b/s/w/ir/cache/builder/src/flutter/flow/layers/backdrop_filter_layer.cc:45:63: error: C-style casts are discouraged; use static_cast/const_cast/reinterpret_cast [google-readability-casting,-warnings-as-errors]
      Layer::AutoPrerollSaveLayerState::Create(context, true, bool(filter_));

❌ Failures for clang-tidy on /b/s/w/ir/cache/builder/src/flutter/impeller/renderer/backend/gles/shader_library_gles.cc:
/b/s/w/ir/cache/builder/src/flutter/impeller/renderer/backend/gles/shader_library_gles.cc:67:45: error: C-style casts are discouraged; use static_cast/const_cast/reinterpret_cast [google-readability-casting,-warnings-as-errors]
    functions[ShaderKey{key_name, stage}] = std::shared_ptr<ShaderFunctionGLES>(

❌ Failures for clang-tidy on /b/s/w/ir/cache/builder/src/flutter/impeller/renderer/backend/vulkan/shader_library_vk.cc:
/b/s/w/ir/cache/builder/src/flutter/impeller/renderer/backend/vulkan/shader_library_vk.cc:88:45: error: C-style casts are discouraged; use static_cast/const_cast/reinterpret_cast [google-readability-casting,-warnings-as-errors]
    functions[ShaderKey{key_name, stage}] = std::shared_ptr<ShaderFunctionVK>(

https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket/8797288801511583249/+/u/test:_lint_host_debug/stdout

(impeller/renderer/backend is building when targeting Linux host?)

Filed flutter/flutter#115518

[jmagman]

google-build-using-namespace warnings: flutter/flutter#115521
google-readability-avoid-underscore-in-googletest-name warnings: #37700
google-default-arguments: flutter/flutter#115522
google-readability-casting: flutter/flutter#115518

[gaaclarke]

@gaaclarke I still see three on Linux host

Yea, sorry I meant at the time of landing that PR they should all have been fixed, but there is nothing stoping future regressions since they are not turned on as errors. We should turn them on as errors.

[jmagman]

but there is nothing stoping future regressions since they are not turned on as errors. We should turn them on as errors.

That's the point of this PR, to make all checks we have on show up as errors, otherwise they won't show up in CI and regressions are inevitable. Those four checks still have issues though, so we will need to turn them off until the issues are fixed.
Draft at #37698

[gaaclarke]

lgtm!

[skia-gold]

Gold has detected about 1 new digest(s) on patchset 4.
View them at https://flutter-engine-gold.skia.org/cl/github/36885