[Xamarin.Android.Build.Tasks] Emit an Error if a custom view cannot be fixed up. #1720

dellis1972 · 2018-05-23T15:31:19Z

Context #1711

When using a custom view within a layout file we replace the
namespace.classname with an md5hash.classname. We do this
by using the acwmap.txt file to make known types onto the
md5 hashed ones. We do this in a case sensitive manner. We
also only support Camel case and lower case type names.

ClassLibrary1.CustomView=md5d6f7135293df7527c983d45d07471c5e.CustomTextView
classlibrary1.CustomView=md5d6f7135293df7527c983d45d07471c5e.CustomTextView

Given that a user is able to type these manually it is highly
probable that typo's will occur. If for example a user types

Classlibrary1.CustomView

(Note the lower case L) this will NOT be fixed up. Instead the user will recieve the
following error at runtime.

FATAL UNHANDLED EXCEPTION: Android.Views.InflateException: Binary XML file line #1: Binary XML file line #1: Error inflating class Classlibrary1.CustomTextView ---> Android.Views.InflateException: Binary XML file line #1: Error inflating class Classlibrary1.CustomTextView ---> Java.Lang.ClassNotFoundException: Didn't find class "Classlibrary1.CustomTextView"

if you are using the control in a number of places this runtime error
does nothing to help track down the problem.

Instead what we should be doing is detecting these issues and emitting
a build error. This will provide the user not only the problem but
also a link to the file causing the probem.

TODO

Fix up the name so it points to the file in Resources not res
Add a Unit test.
Add Error code and document.

jonpryor · 2018-05-24T19:16:23Z

src/Xamarin.Android.Build.Tasks/Tasks/ConvertResourcesCases.cs

+			Log.LogDebugMessage ("  ResourceNameCaseMap: {0}", ResourceNameCaseMap);
+
+			if (ResourceNameCaseMap != null)
+				foreach (var arr in ResourceNameCaseMap.Split (';').Select (l => l.Split ('|')).Where (a => a.Length == 2))


This same code is also in Aapt.cs. We should move this to a utility method.

dellis1972 · 2018-06-19T08:46:19Z

build

dellis1972 · 2018-07-02T10:58:27Z

@jonpryor @jonathanpeppers this should be good to go now.

jonathanpeppers · 2018-07-02T13:17:20Z

src/Xamarin.Android.Build.Tasks/Tasks/ConvertResourcesCases.cs

+							string targetfile = file;
+							if (targetfile.StartsWith (resdir, StringComparison.InvariantCultureIgnoreCase)) {
+								targetfile = file.Substring (resdir.Length).TrimStart (Path.DirectorySeparatorChar);
+								targetfile = resource_name_case_map.ContainsKey (targetfile) ? resource_name_case_map [targetfile] : targetfile;


It might be worth using TryGetValue so the dictionary lookup doesn't happen twice: ContainsKey and indexer.

I'm guessing resource_name_case_map, could be big?

not sure. We would need to add a temp variable since it seems TryGetValue will null the output if it failed.

When this method returns, contains the value associated with the specified key, if the key is found; otherwise, the default value for the type of the value parameter. This parameter is passed uninitialized.

So we can't do

resource_name_case_map.TryGetValue (targetfile, out targetfile);

we would need to do

string temp; if (resource_name_case_map.TryGetValue (targetfile, out temp)) targetfile = temp;

not sure which is better.

ah, stuff it. Gonna change to TryGetValue

I think this could make it a little better:

if (resource_name_case_map.TryGetValue (targetfile, out string temp)) targetfile = temp;

jonathanpeppers · 2018-07-02T13:22:54Z

src/Xamarin.Android.Build.Tasks/Utilities/AndroidResource.cs

+				// we have elements with slightly different casing.
+				// lets issue a error.
+				logMessage (TraceLevel.Error, $"We found a matching key '{matchingKey.Key}' for '{elem.Name.ToString ()}'. But the casing was incorrect. Please correct the casing");
+			}
 			return false;
 		}


It looks like this method calls XElement.Name.ToString () alot, maybe calling it once and storing in a variable would be good?

jonathanpeppers · 2018-07-05T21:43:23Z

src/Xamarin.Android.Build.Tasks/Utilities/AndroidResource.cs

-				elem.Name = acwMap[elem.Name.ToString ()];
+			string name = elem.Name.ToString ();
+			if (acwMap.ContainsKey (name)) {
+				elem.Name = acwMap[name];


This one could also use TryGetValue, sorry I didn't see it before.

jonathanpeppers · 2018-07-05T21:44:05Z

src/Xamarin.Android.Build.Tasks/Tasks/ConvertResourcesCases.cs

+							if (targetfile.StartsWith (resdir, StringComparison.InvariantCultureIgnoreCase)) {
+								targetfile = file.Substring (resdir.Length).TrimStart (Path.DirectorySeparatorChar);
+								string temp;
+								if (resource_name_case_map.TryGetValue (targetfile, out temp))


You might try the new out syntax:

if (resource_name_case_map.TryGetValue (targetfile, out string temp))

jonathanpeppers · 2018-07-05T21:46:50Z

src/Xamarin.Android.Build.Tasks/Utilities/MonoAndroidHelper.cs

+			var result = new Dictionary<string, string> ();
+			if (resourceCaseMap != null) {
+				foreach (var arr in resourceCaseMap.Split (';').Select (l => l.Split ('|')).Where (a => a.Length == 2))
+					result [arr [1]] = arr [0]; // lowercase -> original


This is probably for the future, but I wonder if we could use Span<T> here to make this faster.

This current code looks like it would allocate a lot of strings.

dellis1972 · 2018-07-17T12:30:52Z

@jonathanpeppers fixed up the TryGetValue bits :)

…e fixed up. Context dotnet#1711 When using a custom view within a layout file we replace the `namespace.classname` with an `md5hash.classname`. We do this by using the `acwmap.txt` file to make known types onto the `md5` hashed ones. We do this in a case sensitive manner. We also only support Camel case and lower case type names. ClassLibrary1.CustomView=md5d6f7135293df7527c983d45d07471c5e.CustomTextView classlibrary1.CustomView=md5d6f7135293df7527c983d45d07471c5e.CustomTextView Given that a user is able to type these manually it is highly probable that typo's will occur. If for example a user types Classlibrary1.CustomView this will NOT be fixed up. Instead the user will recieve the following error at runtime. FATAL UNHANDLED EXCEPTION: Android.Views.InflateException: Binary XML file line #1: Binary XML file line #1: Error inflating class Classlibrary1.CustomTextView ---> Android.Views.InflateException: Binary XML file line #1: Error inflating class Classlibrary1.CustomTextView ---> Java.Lang.ClassNotFoundException: Didn't find class "Classlibrary1.CustomTextView" if you are using the control in a number of places this runtime error does nothing to help track down the problem. Instead what we should be doing is detecting these issues and emitting a build error. This will provide the user not only the problem but also a link to the file causing the probem. TODO ---- [ ] Fix up the name so it points to the file in `Resources` not `res` [ ] Add a Unit test. [ ] Add Error code and document.

…e fixed up. (#1720) Context: #1711 When using a custom view within a layout file we replace the `namespace.classname` with an `md5hash.classname`. We do this by using the `acwmap.txt` file to map known types onto the `md5` hashed ones. This is done in a case sensitive manner. We also only support identically-cased and lower-cased namespace/package names: ClassLibrary1.CustomView=md5d6f7135293df7527c983d45d07471c5e.CustomTextView classlibrary1.CustomView=md5d6f7135293df7527c983d45d07471c5e.CustomTextView Given that a user is able to type these manually it is possible that typo's will occur. If for example a user types Classlibrary1.CustomView this will NOT be fixed up, due to the lowercase `l` in `library`. Instead the user will see the following error at runtime. FATAL UNHANDLED EXCEPTION: Android.Views.InflateException: Binary XML file line #1: Binary XML file line #1: Error inflating class Classlibrary1.CustomTextView ---> Android.Views.InflateException: Binary XML file line #1: Error inflating class Classlibrary1.CustomTextView ---> Java.Lang.ClassNotFoundException: Didn't find class "Classlibrary1.CustomTextView" If the control is used in a number of places this runtime error does nothing to help track down the problem. Improve this scenario by detecting these issues and emitting an XA1002 build error. This will not only inform the user about the problem but also provide a link to the file causing the problem.

dellis1972 added the do-not-merge PR should not be merged. label May 23, 2018

jonpryor reviewed May 24, 2018

View reviewed changes

dellis1972 force-pushed the customviewcasing branch from c714a42 to c2563fa Compare June 18, 2018 11:47

dellis1972 removed the do-not-merge PR should not be merged. label Jun 19, 2018

dellis1972 force-pushed the customviewcasing branch from bb55a7c to c818aa0 Compare June 29, 2018 09:23

dellis1972 requested review from jonathanpeppers and jonpryor July 2, 2018 10:58

jonathanpeppers reviewed Jul 2, 2018

View reviewed changes

dellis1972 force-pushed the customviewcasing branch from c818aa0 to 2bbaa3f Compare July 5, 2018 12:32

jonathanpeppers reviewed Jul 5, 2018

View reviewed changes

dellis1972 force-pushed the customviewcasing branch from aee8785 to 9253468 Compare July 17, 2018 12:08

jonathanpeppers approved these changes Jul 17, 2018

View reviewed changes

dellis1972 force-pushed the customviewcasing branch from 021433e to 5ab03c2 Compare July 18, 2018 10:51

dellis1972 force-pushed the customviewcasing branch 2 times, most recently from fb50b19 to c03d6ec Compare August 1, 2018 09:04

dellis1972 added 3 commits August 2, 2018 11:49

Fixed Coded Error problem

6efa469

Fixed docs

1eae104

dellis1972 force-pushed the customviewcasing branch from c03d6ec to 1eae104 Compare August 2, 2018 10:49

jonpryor merged commit 967fe94 into dotnet:master Aug 2, 2018

github-actions bot locked and limited conversation to collaborators Feb 2, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Xamarin.Android.Build.Tasks] Emit an Error if a custom view cannot be fixed up. #1720

[Xamarin.Android.Build.Tasks] Emit an Error if a custom view cannot be fixed up. #1720

dellis1972 commented May 23, 2018 •

edited

Loading

jonpryor May 24, 2018

dellis1972 commented Jun 19, 2018

dellis1972 commented Jul 2, 2018

jonathanpeppers Jul 2, 2018

dellis1972 Jul 5, 2018

dellis1972 Jul 5, 2018

jonathanpeppers Jul 5, 2018

jonathanpeppers Jul 2, 2018

jonathanpeppers Jul 5, 2018

jonathanpeppers Jul 5, 2018

jonathanpeppers Jul 5, 2018

dellis1972 commented Jul 17, 2018

[Xamarin.Android.Build.Tasks] Emit an Error if a custom view cannot be fixed up. #1720

[Xamarin.Android.Build.Tasks] Emit an Error if a custom view cannot be fixed up. #1720

Conversation

dellis1972 commented May 23, 2018 • edited Loading

TODO

Choose a reason for hiding this comment

dellis1972 commented Jun 19, 2018

dellis1972 commented Jul 2, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dellis1972 commented Jul 17, 2018

dellis1972 commented May 23, 2018 •

edited

Loading