From 94438d977b0ad96128c8c4542aa8864fc8344236 Mon Sep 17 00:00:00 2001 From: Jenn Magder Date: Wed, 19 Oct 2022 17:54:33 -0700 Subject: [PATCH 1/4] Only run clang-tidy warning checks reported as errors --- tools/clang_tidy/lib/src/command.dart | 3 +-- tools/clang_tidy/test/clang_tidy_test.dart | 18 ++++++++++-------- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/tools/clang_tidy/lib/src/command.dart b/tools/clang_tidy/lib/src/command.dart index 278d6da154b9c..f2132b2666934 100644 --- a/tools/clang_tidy/lib/src/command.dart +++ b/tools/clang_tidy/lib/src/command.dart @@ -134,8 +134,7 @@ class Command { WorkerJob createLintJob(Options options) { final List args = [ filePath, - if (options.warningsAsErrors != null) - '--warnings-as-errors=${options.warningsAsErrors}', + '--warnings-as-errors=${options.warningsAsErrors ?? '*'}', if (options.checks != null) options.checks!, if (options.fix) ...[ diff --git a/tools/clang_tidy/test/clang_tidy_test.dart b/tools/clang_tidy/test/clang_tidy_test.dart index 73129a0f99665..13b2e08188ae1 100644 --- a/tools/clang_tidy/test/clang_tidy_test.dart +++ b/tools/clang_tidy/test/clang_tidy_test.dart @@ -379,19 +379,21 @@ Future main(List args) async { final WorkerJob jobNoFix = command.createLintJob(noFixOptions); expect(jobNoFix.command[0], endsWith('../../buildtools/mac-x64/clang/bin/clang-tidy')); expect(jobNoFix.command[1], endsWith(filePath.replaceAll('/', io.Platform.pathSeparator))); - expect(jobNoFix.command[2], '--'); - expect(jobNoFix.command[3], ''); - expect(jobNoFix.command[4], endsWith(filePath)); + expect(jobNoFix.command[2], '--warnings-as-errors=*'); + expect(jobNoFix.command[3], '--'); + expect(jobNoFix.command[4], ''); + expect(jobNoFix.command[5], endsWith(filePath)); final Options fixOptions = Options(buildCommandsPath: io.File('.'), fix: true); final WorkerJob jobWithFix = command.createLintJob(fixOptions); expect(jobWithFix.command[0], endsWith('../../buildtools/mac-x64/clang/bin/clang-tidy')); expect(jobWithFix.command[1], endsWith(filePath.replaceAll('/', io.Platform.pathSeparator))); - expect(jobWithFix.command[2], '--fix'); - expect(jobWithFix.command[3], '--format-style=file'); - expect(jobWithFix.command[4], '--'); - expect(jobWithFix.command[5], ''); - expect(jobWithFix.command[6], endsWith(filePath)); + expect(jobWithFix.command[2], '--warnings-as-errors=*'); + expect(jobWithFix.command[3], '--fix'); + expect(jobWithFix.command[4], '--format-style=file'); + expect(jobWithFix.command[5], '--'); + expect(jobWithFix.command[6], ''); + expect(jobWithFix.command[7], endsWith(filePath)); }); test('Command getLintAction flags third_party files', () async { From 062366882b48bd682aaf618cc4550c5d97d30429 Mon Sep 17 00:00:00 2001 From: Jenn Magder Date: Wed, 16 Nov 2022 18:30:18 -0800 Subject: [PATCH 2/4] Remove clang-tidy warning list --- .clang-tidy | 19 +++---------------- 1 file changed, 3 insertions(+), 16 deletions(-) diff --git a/.clang-tidy b/.clang-tidy index 7947394ce3950..2fc0e5cb81696 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -7,8 +7,11 @@ clang-diagnostic-*,\ google-*,\ modernize-use-default-member-init,\ readability-identifier-naming,\ +-google-build-using-namespace,\ +-google-default-arguments,\ -google-objc-global-variable-declaration,\ -google-objc-avoid-throwing-exception,\ +-google-readability-casting,\ -clang-analyzer-nullability.NullPassedToNonnull,\ -clang-analyzer-nullability.NullablePassedToNonnull,\ -clang-analyzer-nullability.NullReturnedFromNonnull,\ @@ -16,22 +19,6 @@ readability-identifier-naming,\ performance-move-const-arg,\ performance-unnecessary-value-param" -# Only warnings treated as errors are reported -# in the "ci/lint.sh" script and pre-push git hook. -# Add checks when all warnings are fixed -# to prevent new warnings being introduced. -# https://github.com/flutter/flutter/issues/93279 -# Note: There are platform specific warnings as errors in -# //ci/lint.sh -WarningsAsErrors: "bugprone-use-after-move,\ -clang-analyzer-*,\ -readability-identifier-naming,\ -clang-diagnostic-*,\ -google-objc-*,\ -google-explicit-constructor,\ -performance-move-const-arg,\ -performance-unnecessary-value-param" - CheckOptions: - key: modernize-use-default-member-init.UseAssignment value: true From 2fa19e95d4ffac84cc53887473dbcf9fc1ed8f4c Mon Sep 17 00:00:00 2001 From: Jenn Magder Date: Wed, 16 Nov 2022 18:33:09 -0800 Subject: [PATCH 3/4] Turn on clang tidy error for underscores in Google tests --- .../display_list_image_filter_unittests.cc | 2 +- flow/raster_cache_unittests.cc | 6 +-- .../embedder/tests/embedder_unittests.cc | 46 +++++++++++-------- 3 files changed, 32 insertions(+), 22 deletions(-) diff --git a/display_list/display_list_image_filter_unittests.cc b/display_list/display_list_image_filter_unittests.cc index c447d52e81586..95807c061666b 100644 --- a/display_list/display_list_image_filter_unittests.cc +++ b/display_list/display_list_image_filter_unittests.cc @@ -653,7 +653,7 @@ TEST(DisplayListImageFilter, ComposeBoundsWithUnboundedInnerAndOuter) { } // See https://github.com/flutter/flutter/issues/108433 -TEST(DisplayListImageFilter, Issue_108433) { +TEST(DisplayListImageFilter, Issue108433) { auto input_bounds = SkIRect::MakeLTRB(20, 20, 80, 80); auto sk_filter = SkColorFilters::Blend(SK_ColorRED, SkBlendMode::kSrcOver); diff --git a/flow/raster_cache_unittests.cc b/flow/raster_cache_unittests.cc index d72b91abf3b1b..47f3e0b10e5f3 100644 --- a/flow/raster_cache_unittests.cc +++ b/flow/raster_cache_unittests.cc @@ -709,7 +709,7 @@ TEST(RasterCache, RasterCacheKeySameType) { ASSERT_EQ(map[layer_children_third_key], 300); } -TEST(RasterCache, RasterCacheKeyID_Equal) { +TEST(RasterCache, RasterCacheKeyIDEqual) { RasterCacheKeyID first = RasterCacheKeyID(1, RasterCacheKeyType::kLayer); RasterCacheKeyID second = RasterCacheKeyID(2, RasterCacheKeyType::kLayer); RasterCacheKeyID third = @@ -729,7 +729,7 @@ TEST(RasterCache, RasterCacheKeyID_Equal) { ASSERT_NE(fifth, sixth); } -TEST(RasterCache, RasterCacheKeyID_HashCode) { +TEST(RasterCache, RasterCacheKeyIDHashCode) { uint64_t foo = 1; uint64_t bar = 2; RasterCacheKeyID first = RasterCacheKeyID(foo, RasterCacheKeyType::kLayer); @@ -763,7 +763,7 @@ TEST(RasterCache, RasterCacheKeyID_HashCode) { using RasterCacheTest = SkiaGPUObjectLayerTest; -TEST_F(RasterCacheTest, RasterCacheKeyID_LayerChildrenIds) { +TEST_F(RasterCacheTest, RasterCacheKeyIDLayerChildrenIds) { auto layer = std::make_shared(); const SkPath child_path = SkPath().addRect(SkRect::MakeWH(5.0f, 5.0f)); diff --git a/shell/platform/embedder/tests/embedder_unittests.cc b/shell/platform/embedder/tests/embedder_unittests.cc index 74f6c4a563731..36d4057e20786 100644 --- a/shell/platform/embedder/tests/embedder_unittests.cc +++ b/shell/platform/embedder/tests/embedder_unittests.cc @@ -1629,44 +1629,54 @@ static void expectSoftwareRenderingOutputMatches( std::vector(bytes, bytes + sizeof(T))); } -#define SW_PIXFMT_TEST_F(dart_entrypoint, pixfmt, matcher) \ - TEST_F(EmbedderTest, \ - SoftwareRenderingPixelFormats_##dart_entrypoint##_##pixfmt) { \ +#define SW_PIXFMT_TEST_F(test_name, dart_entrypoint, pixfmt, matcher) \ + TEST_F(EmbedderTest, SoftwareRenderingPixelFormats##test_name) { \ expectSoftwareRenderingOutputMatches(*this, #dart_entrypoint, pixfmt, \ matcher); \ } // Don't test the pixel formats that contain padding (so an X) and the kNative32 // pixel format here, so we don't add any flakiness. -SW_PIXFMT_TEST_F(draw_solid_red, kRGB565, (uint16_t)0xF800); -SW_PIXFMT_TEST_F(draw_solid_red, kRGBA4444, (uint16_t)0xF00F); -SW_PIXFMT_TEST_F(draw_solid_red, +SW_PIXFMT_TEST_F(RedRGBA565xF800, draw_solid_red, kRGB565, (uint16_t)0xF800); +SW_PIXFMT_TEST_F(RedRGBA4444xF00F, draw_solid_red, kRGBA4444, (uint16_t)0xF00F); +SW_PIXFMT_TEST_F(RedRGBA8888xFFx00x00xFF, + draw_solid_red, kRGBA8888, (std::vector{0xFF, 0x00, 0x00, 0xFF})); -SW_PIXFMT_TEST_F(draw_solid_red, +SW_PIXFMT_TEST_F(RedBGRA8888x00x00xFFxFF, + draw_solid_red, kBGRA8888, (std::vector{0x00, 0x00, 0xFF, 0xFF})); -SW_PIXFMT_TEST_F(draw_solid_red, kGray8, (uint8_t)0x36); - -SW_PIXFMT_TEST_F(draw_solid_green, kRGB565, (uint16_t)0x07E0); -SW_PIXFMT_TEST_F(draw_solid_green, kRGBA4444, (uint16_t)0x0F0F); -SW_PIXFMT_TEST_F(draw_solid_green, +SW_PIXFMT_TEST_F(RedGray8x36, draw_solid_red, kGray8, (uint8_t)0x36); + +SW_PIXFMT_TEST_F(GreenRGB565x07E0, draw_solid_green, kRGB565, (uint16_t)0x07E0); +SW_PIXFMT_TEST_F(GreenRGBA4444x0F0F, + draw_solid_green, + kRGBA4444, + (uint16_t)0x0F0F); +SW_PIXFMT_TEST_F(GreenBGRA8888x00xFFx00xFF, + draw_solid_green, kRGBA8888, (std::vector{0x00, 0xFF, 0x00, 0xFF})); -SW_PIXFMT_TEST_F(draw_solid_green, +SW_PIXFMT_TEST_F(GreenBGRA8888x00xFFx00xFF, + draw_solid_green, kBGRA8888, (std::vector{0x00, 0xFF, 0x00, 0xFF})); -SW_PIXFMT_TEST_F(draw_solid_green, kGray8, (uint8_t)0xB6); +SW_PIXFMT_TEST_F(GreenGray8xB6, draw_solid_green, kGray8, (uint8_t)0xB6); -SW_PIXFMT_TEST_F(draw_solid_blue, kRGB565, (uint16_t)0x001F); -SW_PIXFMT_TEST_F(draw_solid_blue, kRGBA4444, (uint16_t)0x00FF); +SW_PIXFMT_TEST_F(BlueRGB565x001F, draw_solid_blue, kRGB565, (uint16_t)0x001F); +SW_PIXFMT_TEST_F(BlueRGBA4444x00FF, + draw_solid_blue, + kRGBA4444, + (uint16_t)0x00FF); SW_PIXFMT_TEST_F(draw_solid_blue, kRGBA8888, (std::vector{0x00, 0x00, 0xFF, 0xFF})); -SW_PIXFMT_TEST_F(draw_solid_blue, +SW_PIXFMT_TEST_F(BlueBGRA8888xFFx00x00xFF, + draw_solid_blue, kBGRA8888, (std::vector{0xFF, 0x00, 0x00, 0xFF})); -SW_PIXFMT_TEST_F(draw_solid_blue, kGray8, (uint8_t)0x12); +SW_PIXFMT_TEST_F(BlueGray8x12, draw_solid_blue, kGray8, (uint8_t)0x12); //------------------------------------------------------------------------------ // Key Data From eda88726ca44d595b9223c46a8a4ba98d06d4d28 Mon Sep 17 00:00:00 2001 From: Jenn Magder Date: Wed, 16 Nov 2022 18:33:09 -0800 Subject: [PATCH 4/4] Turn on clang tidy error for underscores in Google tests --- shell/platform/embedder/tests/embedder_unittests.cc | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/shell/platform/embedder/tests/embedder_unittests.cc b/shell/platform/embedder/tests/embedder_unittests.cc index 36d4057e20786..e9616336a598a 100644 --- a/shell/platform/embedder/tests/embedder_unittests.cc +++ b/shell/platform/embedder/tests/embedder_unittests.cc @@ -1654,7 +1654,7 @@ SW_PIXFMT_TEST_F(GreenRGBA4444x0F0F, draw_solid_green, kRGBA4444, (uint16_t)0x0F0F); -SW_PIXFMT_TEST_F(GreenBGRA8888x00xFFx00xFF, +SW_PIXFMT_TEST_F(GreenRGBA8888x00xFFx00xFF, draw_solid_green, kRGBA8888, (std::vector{0x00, 0xFF, 0x00, 0xFF})); @@ -1669,7 +1669,8 @@ SW_PIXFMT_TEST_F(BlueRGBA4444x00FF, draw_solid_blue, kRGBA4444, (uint16_t)0x00FF); -SW_PIXFMT_TEST_F(draw_solid_blue, +SW_PIXFMT_TEST_F(BlueRGBA8888x00x00xFFxFF, + draw_solid_blue, kRGBA8888, (std::vector{0x00, 0x00, 0xFF, 0xFF})); SW_PIXFMT_TEST_F(BlueBGRA8888xFFx00x00xFF,