[Warning] Eliminated AnnotateFormatMethod warnings #7353

hellosagar · 2022-06-04T21:01:21Z

Overview

Eliminated the AnnotateFormatMethod across the entire project.

Proposed Changes

Added the changes in the following files to fix the error-prone warning

resources/src/main/java/org/robolectric/res/android/Util.java

resources/src/main/java/org/robolectric/res/android/ResXMLParser.java

utzcoz · 2022-06-11T14:37:49Z

@hellosagar Look like new commit to applying google java format generates a reasonable change lines, instead of previous multi thousands lines.

hellosagar · 2022-06-11T14:41:57Z

@hellosagar Look like new commit to applying google java format generates a reasonable change lines, instead of previous multi thousands lines.

Yes :)

resources/src/main/java/org/robolectric/res/android/AttributeResolution10.java

utzcoz · 2022-06-11T14:44:17Z

There are some formats don't look great, although it has passed checking. And I have wrote some comments for some wrong formats, I think.

hellosagar · 2022-06-11T14:46:20Z

um.. lme check that locally on my system

hellosagar · 2022-06-11T16:23:15Z

@utzcoz you are right, I little surprised how it passed in the CI but mainly AttributeResolution10.java contains a lot of weird indentation, Ive reformat it locally, pushing it again

utzcoz · 2022-06-11T16:28:18Z

Why formatted change lines increase a lot again?

hellosagar · 2022-06-11T16:30:32Z

Why formatted change lines increase a lot again?

because I open that file and pressed the cmd + ctrl + L which performed the google style format as I've their plugin installed.

TDLR: Used the plugin to reformat the file according to the standards

utzcoz · 2022-06-11T16:37:07Z

It's not recommend to add not related formatted lines. Could you format with command in wiki? We just need add formatted lines that affected by this PR.

hellosagar · 2022-06-11T16:41:42Z

Okay, lme undo this

hellosagar · 2022-06-11T16:47:59Z

pushed again according to the wiki, but in that ALGOI method has a weird indentation

utzcoz · 2022-06-11T16:49:55Z

Could you fix them manually one by one and check locally whether they can pass google format checking?

hellosagar · 2022-06-11T16:51:04Z

Could you fix them manually one by one and check locally whether they can pass google format checking?

I tried it first before pushing 😅, but it shows a google format error.

utzcoz · 2022-06-11T16:52:44Z

Okay, I will check and verify it tomorrow.

hellosagar · 2022-06-11T16:54:13Z

After manually, changing the indentation for ALGOI, this log is generated

❯ /bin/zsh /Users/sagarkhurana/checkJavaFormat.sh
Please run google-java-format on the changes in this pull request
--- resources/src/main/java/org/robolectric/res/android/AttributeResolution10.java      (before formatting)
+++ resources/src/main/java/org/robolectric/res/android/AttributeResolution10.java      (after formatting)
@@ -94,9 +94,9 @@
             int src_values_length, int[] attrs,
             int attrs_length, int[] out_values, int[] out_indices) {
         if (kDebugStyles) {
-            ALOGI(
-                "APPLY STYLE: theme=0x%s defStyleAttr=0x%x defStyleRes=0x%x",
-                theme, def_style_attr, def_style_res);
+      ALOGI(
+          "APPLY STYLE: theme=0x%s defStyleAttr=0x%x defStyleRes=0x%x",
+          theme, def_style_attr, def_style_res);
         }
 
         CppAssetManager2 assetmanager = theme.GetAssetManager();
@@ -160,7 +160,7 @@
                     type_set_flags = def_style_flags.get();
                     value = entry.value;
                     if (kDebugStyles) {
-                        ALOGI("-> From def style: type=0x%x, data=0x%08x", value.dataType, value.data);
+            ALOGI("-> From def style: type=0x%x, data=0x%08x", value.dataType, value.data);
                     }
                 }
             }
@@ -193,7 +193,7 @@
                         cookie = new_cookie;
                     }
                     if (kDebugStyles) {
-                        ALOGI("-> Resolved theme: type=0x%x, data=0x%08x", value.dataType, value.data);
+            ALOGI("-> Resolved theme: type=0x%x, data=0x%08x", value.dataType, value.data);
                     }
                 }
             }
@@ -333,7 +333,7 @@
                 xml_parser.getAttributeValue(xml_attr_idx, value);
                 type_set_flags.set(style_flags.get());
                 if (kDebugStyles) {
-                    ALOGI("-> From XML: type=0x%x, data=0x%08x", value.get().dataType, value.get().data);
+          ALOGI("-> From XML: type=0x%x, data=0x%08x", value.get().dataType, value.get().data);
                 }
             }
 
@@ -347,9 +347,9 @@
                     value.set(entry.value);
                     source_style_resid = entry.style;
                     if (kDebugStyles) {
-                        ALOGI(
-                            "-> From style: type=0x%x, data=0x%08x, style=0x%08x",
-                            value.get().dataType, value.get().data, entry.style);
+            ALOGI(
+                "-> From style: type=0x%x, data=0x%08x, style=0x%08x",
+                value.get().dataType, value.get().data, entry.style);
                     }
                 }
             }
@@ -364,9 +364,9 @@
 
                     value.set(entry.value);
                     if (kDebugStyles) {
-                        ALOGI(
-                            "-> From def style: type=0x%x, data=0x%08x, style=0x%08x",
-                            value.get().dataType, value.get().data, entry.style);
+            ALOGI(
+                "-> From def style: type=0x%x, data=0x%08x, style=0x%08x",
+                value.get().dataType, value.get().data, entry.style);
                     }
                     source_style_resid = entry.style;
                 }
@@ -382,14 +382,14 @@
                 }
 
                 if (kDebugStyles) {
-                    ALOGI("-> Resolved attr: type=0x%x, data=0x%08x", value.get().dataType, value.get().data);
+          ALOGI("-> Resolved attr: type=0x%x, data=0x%08x", value.get().dataType, value.get().data);
                 }
             } else if (value.get().data != Res_value.DATA_NULL_EMPTY) {
                 // If we still don't have a value for this attribute, try to find it in the theme!
                 ApkAssetsCookie new_cookie = theme.GetAttribute(cur_ident, value, type_set_flags);
                 if (new_cookie.intValue() != kInvalidCookie) {
                     if (kDebugStyles) {
-                        ALOGI("-> From theme: type=0x%x, data=0x%08x", value.get().dataType, value.get().data);
+            ALOGI("-> From theme: type=0x%x, data=0x%08x", value.get().dataType, value.get().data);
                     }
                     new_cookie =
                             assetmanager.ResolveReference(new_cookie, value, config, type_set_flags, resid);
@@ -398,9 +398,9 @@
                     }
 
                     if (kDebugStyles) {
-                        ALOGI(
-                            "-> Resolved theme: type=0x%x, data=0x%08x",
-                            value.get().dataType, value.get().data);
+            ALOGI(
+                "-> Resolved theme: type=0x%x, data=0x%08x",
+                value.get().dataType, value.get().data);
                     }
                 }
             }
@@ -415,9 +415,9 @@
             }
 
             if (kDebugStyles) {
-                ALOGI(
-                    "Attribute 0x%08x: type=0x%x, data=0x%08x",
-                    cur_ident, value.get().dataType, value.get().data);
+        ALOGI(
+            "Attribute 0x%08x: type=0x%x, data=0x%08x",
+            cur_ident, value.get().dataType, value.get().data);
             }
 
             // Write the final value back to Java.

utzcoz · 2022-08-06T14:29:31Z

@hellosagar could you rebase and squash commits for this PR? I think wrong formatting are generated by wrong HEAD when using google-java-format locally. We have analyzed this problem before. Maybe you can get correct code formatting after rebasing, squashing, and running google-java-format with correct command locally again.

hellosagar · 2022-08-09T16:26:31Z

I've ran the formatting locally and pushed it again

hellosagar · 2022-08-30T13:50:25Z

@utzcoz any update after I pushed the new commit?

utzcoz · 2022-08-31T02:15:52Z

@hellosagar Please fix incorrect format problem and rebase it to latest master.

hellosagar · 2022-08-31T19:04:41Z

:integration_tests:androidx_test:connectedDebugAndroidTest test is even failing in master as well

Below is the log generated from master branch

resources/src/main/java/org/robolectric/res/android/AttributeResolution10.java

hellosagar · 2022-09-01T16:51:41Z

@utzcoz I've added a separate commit for easy review, later I'll squash it. Please have a look at it

utzcoz · 2022-09-02T02:37:16Z

@utzcoz I've added a separate commit for easy review, later I'll squash it. Please have a look at it

You can squash it. Hi @hoisie What about skipping format error for this special case?

hellosagar · 2022-09-03T07:54:33Z

Squashed :)

resources/src/main/java/org/robolectric/res/android/Util.java

utzcoz · 2022-09-13T11:23:09Z

@hellosagar Could you enable kDebugStringPoolNoisy and build and run tests locally to check whether your changes can work without error? The default value of kDebugStringPoolNoisy is false, and I think current CI will not run into some code you modified.

hellosagar · 2022-09-17T06:39:20Z

Locally all checks are passing, without enabling the kDebugStringPoolNoisy in ResTable and ResXMLParser

and this is the output after enabling the kDebugStringPoolNoisy in ResTable and ResXMLParser

utzcoz · 2022-09-18T07:32:53Z

Hi @hoisie , because there are strange code formatting generated by google-java-format, so I leave format checking error here, and merge this PR firstly.

hellosagar force-pushed the annotate-format-method-warning-fix branch from fd89861 to afd6132 Compare June 4, 2022 22:30

utzcoz reviewed Jun 5, 2022

View reviewed changes

resources/src/main/java/org/robolectric/res/android/Util.java Show resolved Hide resolved

resources/src/main/java/org/robolectric/res/android/ResXMLParser.java Outdated Show resolved Hide resolved

hellosagar force-pushed the annotate-format-method-warning-fix branch 4 times, most recently from bc492a8 to ac9c16b Compare June 5, 2022 19:47

utzcoz reviewed Jun 11, 2022

View reviewed changes

resources/src/main/java/org/robolectric/res/android/AttributeResolution10.java Outdated Show resolved Hide resolved

resources/src/main/java/org/robolectric/res/android/AttributeResolution10.java Outdated Show resolved Hide resolved

hellosagar force-pushed the annotate-format-method-warning-fix branch from ac9c16b to 373a40c Compare June 11, 2022 16:23

hellosagar force-pushed the annotate-format-method-warning-fix branch from 373a40c to 388645e Compare June 11, 2022 16:47

hellosagar force-pushed the annotate-format-method-warning-fix branch from 388645e to 56971fb Compare August 9, 2022 16:25

hellosagar force-pushed the annotate-format-method-warning-fix branch from 56971fb to 01a55d1 Compare August 31, 2022 16:50

utzcoz requested changes Sep 1, 2022

View reviewed changes

resources/src/main/java/org/robolectric/res/android/AttributeResolution10.java Outdated Show resolved Hide resolved

hellosagar force-pushed the annotate-format-method-warning-fix branch 6 times, most recently from 0c4940f to 2b875a8 Compare September 3, 2022 06:42

utzcoz reviewed Sep 4, 2022

View reviewed changes

resources/src/main/java/org/robolectric/res/android/Util.java Show resolved Hide resolved

hellosagar force-pushed the annotate-format-method-warning-fix branch from 2b875a8 to 3942174 Compare September 10, 2022 11:44

fix: AnnotateFormatMethod and FormatStringAnnotation warning

049cfe2

hellosagar force-pushed the annotate-format-method-warning-fix branch from 3942174 to 049cfe2 Compare September 12, 2022 14:08

utzcoz requested a review from hoisie September 18, 2022 07:32

utzcoz merged commit 3a72ce4 into robolectric:master Sep 18, 2022

utzcoz mentioned this pull request Sep 18, 2022

Remove 0x prefix for string format log #7617

Merged

[Warning] Eliminated AnnotateFormatMethod warnings #7353

[Warning] Eliminated AnnotateFormatMethod warnings #7353

Uh oh!

Conversation

hellosagar commented Jun 4, 2022

Overview

Proposed Changes

Uh oh!

Uh oh!

Uh oh!

utzcoz commented Jun 11, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hellosagar commented Jun 11, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

utzcoz commented Jun 11, 2022

Uh oh!

hellosagar commented Jun 11, 2022

Uh oh!

hellosagar commented Jun 11, 2022

Uh oh!

utzcoz commented Jun 11, 2022

Uh oh!

hellosagar commented Jun 11, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

utzcoz commented Jun 11, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hellosagar commented Jun 11, 2022

Uh oh!

hellosagar commented Jun 11, 2022

Uh oh!

utzcoz commented Jun 11, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hellosagar commented Jun 11, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

utzcoz commented Jun 11, 2022

Uh oh!

hellosagar commented Jun 11, 2022

Uh oh!

utzcoz commented Aug 6, 2022

Uh oh!

hellosagar commented Aug 9, 2022

Uh oh!

hellosagar commented Aug 30, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

utzcoz commented Aug 31, 2022

Uh oh!

hellosagar commented Aug 31, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

hellosagar commented Sep 1, 2022

Uh oh!

utzcoz commented Sep 2, 2022

Uh oh!

hellosagar commented Sep 3, 2022

Uh oh!

Uh oh!

utzcoz commented Sep 13, 2022

Uh oh!

hellosagar commented Sep 17, 2022

Uh oh!

utzcoz commented Sep 18, 2022

Uh oh!

Uh oh!

utzcoz commented Jun 11, 2022 •

edited

Loading

hellosagar commented Jun 11, 2022 •

edited

Loading

hellosagar commented Jun 11, 2022 •

edited

Loading

utzcoz commented Jun 11, 2022 •

edited

Loading

utzcoz commented Jun 11, 2022 •

edited

Loading

hellosagar commented Jun 11, 2022 •

edited

Loading

hellosagar commented Aug 30, 2022 •

edited

Loading

hellosagar commented Aug 31, 2022 •

edited

Loading