Nullable is Inconsistent

[gmck]

Android application type

Android for .NET (net6.0-android, etc.)

Affected platform version

VS 2022 17.5.0 Prev 1

Description

After getting my latest test app built and running in release mode, I then went back and enabled Nullable.
I’m now getting inconsistent results regarding replacing code such as if (Build.VERSION.SdkInt >= BuildVersionCodes.S)) with if (OperatingSystem.IsAndroidVersionAtLeast(31)).

See issue #7586 now closed and @jonathanpeppers reply.

As I progressed through all the .cs files modifying code for nullable, I was replacing code such as the above with OperatingSystem.IsAndroidVersionAtLeast(xx) as I went and the squiggles would go away etc. and I finished up one night with everything corrected except for one file SettingsFragment, which I knew had a number of these to be fixed.

So the next day I started to fix the SettingsFragment which needed other nullable type corrections as well. So I fixed all the other stuff leaving the OperatingSystem.IsAndroidVersionAtLeast(xx) to last.

As I started to fix them, I noticed that a number of them weren’t showing the usual squiggles and they were also building without any warnings.

So now I have a number of them where the new code is commented out, the old code is in play, and I still don’t have any warnings. I’ve tried cleaning and deleting the obj and bin folders, but I can’t make the warnings return using the old code.

Another example that resulted in a crash at runtime. (HelperExplanationDialogFragment)

BottomSheetBehavior? bottomSheetBehavior = BottomSheetBehavior.From((View)view!.Parent!); //OK
BottomSheetBehavior? bottomSheetBehavior = BottomSheetBehavior.From(view!.Parent is View);// Crashed at runtime

The last line was the suggested fix by the IDE.

Error from the log:

attempt to pass an instance of java.lang.Boolean as argument 1 to com.google.android.material.bottomsheet.BottomSheetBehavior com.google.android.material.bottomsheet.BottomSheetBehavior.from(android.view.View)

A final example is probably more of a how-to question.

How to correct the following?

Toast.MakeText(Activity, Resources.GetString(Resource.String.toast_message), ToastLength.Long).Show();

You can do the following, but would you want to?

Toast? toast = Toast.MakeText(Activity!, Resources.GetString(Resource.String.toast_message), ToastLength.Long);
 toast!.Show();

Test app https://github.com/gmck/NavigationGraph7Net7

Steps to Reproduce

See above.

Did you find any workaround?

No

Relevant log output

See above

[dellis1972]

@gmck I'm not sure, I'm gonna ask someone with a bit more experience on Nullables to take a look at this

[jonpryor]

@gmck: regarding your runtime crash, it is the result of a combination of factors:

1. generator has a bug wherein generic type parameters are bound as Java.Lang.Object and not the closest matching type constraint on the type parameter; see Use Java generic type parameters constraints java-interop#669
   Because of this, static <V extends View> BottomSheetBehavior<V> BottomSheetBehavior.from(V) is bound as BottomSheetBehavior.From(Java.Lang.Object), not as BottomSheetBehavior.From(Android.Views.View).

2. Java.Lang.Object has an implicit conversion from bool, which "boxes" boolean values into Java.Lang.Boolean values.

3. The fix suggested by the IDE is either wrong, or mis-interpreted. I have not tried to repro this.

In particular, the type of the expression e is T is of type bool. What your "IDE-suggested" code is actually doing is equivalent to:

bool v1 = view!.Parent is View;
var  v2 = new Java.Lang.Boolean(v1); // via "bool to Java.Lang.Object" implicit conversion
BottomSheetBehavior? bottomSheetBehavior = BottomSheetBehavior.From(v2);

What would work is if you used the as operator, e as T, which returns an expression of type T:

BottomSheetBehavior? bottomSheetBehavior = BottomSheetBehavior.From(view!.Parent as View);

Regarding your Toast.MakeText() question, I don't think there's a better way, unless you want to really ensure than Activity isn't null:

Toast toast = Toast.MakeText(
    Activity ?? throw new InvalidOperationException("Activity is null!"),
    Resources.GetString(Resource.String.toast_message),
    ToastLength.Long
) ?? throw new InvalidOperationException("Toast.MakeText returned null!");
toast.Show();

[ghost]

Hi @gmck. We have added the "need-info" label to this issue, which indicates that we have an open question for you before we can take further action. This issue will be closed automatically in 7 days if we do not hear back from you by then - please feel free to re-open it if you come back to this issue after that time.

[jonpryor]

@gmck: are there any remaining questions or issues to address? Thanks!

[gmck]

@jonpryor

What would work is if you used the as operator, e as T, which returns an expression of type T:
BottomSheetBehavior? bottomSheetBehavior = BottomSheetBehavior.From(view!.Parent as View);

My original code (xamarin.android version) was From(view.Parent as View) therefore my first effort was as you have written, but that turned green.

If I paste your code example from here - it still turns green.

However, casting it is ok.
BottomSheetBehavior? bottomSheetBehavior = BottomSheetBehavior.From((View)view!.Parent!); //OK

Isn't (View) the equivalent of "as View"?

are there any remaining questions or issues to address? Thanks!

if (Build.VERSION.SdkInt >= BuildVersionCodes.S)) with if (OperatingSystem.IsAndroidVersionAtLeast(31))
My question re the above in this thread.

Also, issues raised in #7570 re Animations changes between Xamarin.AndroidX.Navigation.Ui from 2.5.1 to 2.5.2 and menu deprecations in #611 in AndroidX

[jonpryor]

@gmck asked:

Isn't (View) the equivalent of as View?

Not quite; the cast in (View)view!.Parent! will throw InvalidCastException if the runtime type of view.Parent is not a View or View subclass. view!.Parent as View will return null if the runtime type of view.Parent is not a View or View subclass.

Meanwhile, the source for BottomSheetBehavior.from() specifies that the argument be a non-null value:

public static <V extends View> BottomSheetBehavior<V> from(@NonNull V view);

Our binding infrastructure "forwards" this, so the binding BottomSheetBehavior.From(Java.Lang.Object view) requires that view be a non-nullable reference. Because e as T can return null, this is why your IDE is warning about BottomSheetBehabior.From(view.Parent as View).

Using an explicit cast is the correct approach, certainly more-so than using e is T.

[jonpryor]

@gmck asked:

if (Build.VERSION.SdkInt >= BuildVersionCodes.S)) with if (OperatingSystem.IsAndroidVersionAtLeast(31))
My question re the above in this thread.

I'm sorry, but I do not understand the question. #7586 was closed because the question was answered -- the CA… checking code doesn't know about Build.VERSION.SdkInt-based checks but does know about OperatingSystem.IsAndroidVersionAtLeast()-based checks -- and I see that the gmck/NavigationGraph7Net7 test app is using OperatingSystem.IsAndroidVersionAtLeast(), so what is the question?

What IDE are you using?

Relatedly, if I edit Dialogs/HelperExplanationDialogFragment.cs line 125 to call BottomSheetBehavior.From(view?.Parent as View), I do see a "green squiggle", and when I view Visual Studio's "Quick Actions and Refactorings…" for the expression, the only suggested fix is to insert a cast. I see no option for view?.Parent is View.

I'm very likely using something wrong, but I do not see anything "green" when I use Visual Studio and e.g. edit Fragments/SettingsFragment.cs line 31 to use the older version check:

if (Build.VERSION.SdkInt >= BuildVersionCodes.S) 
{
    …
}

I don't see anything in that block which would require an API-31 check either. Why does that version check exist?

As I started to fix them, I noticed that a number of them weren’t showing the usual squiggles and they were also building without any warnings.

Is there a particular block of code that you feel should be emitting a warning, and isn't, and an explanation for why you would expect a warning?

You may be observing dotnet/java-interop#1037.

Questions in other issues should be handled in those issues, not here.

[ghost]

Hi @gmck. We have added the "need-info" label to this issue, which indicates that we have an open question for you before we can take further action. This issue will be closed automatically in 7 days if we do not hear back from you by then - please feel free to re-open it if you come back to this issue after that time.

[gmck]

@jonpryor

I'm very likely using something wrong, but I do not see anything "green" when I use Visual Studio and e.g. edit Fragments/SettingsFragment.cs line 31 to use the older version check:

if (Build.VERSION.SdkInt >= BuildVersionCodes.S)
{
…
}
I don't see anything in that block which would require an API-31 check either. Why does that version check exist?

It's required because the PreferenceChange calls SetDefaultNightMode12 which calls a function that is only available in Android12+. Therefore I wanted to disable the Preference for devices below 12.

I've been able to reproduce the green squiggly in a new project. I just imported the SettingsFragment and built it with Nullable disabled. However the squiggly is not in OnCreatePreferences, but in SetDefaultNightMode12 in the following

//if (Build.VERSION.SdkInt >= BuildVersionCodes.S)
                uiModeManager.SetApplicationNightMode((int)uiNightMode);  // Only available in Android 12 and above.

With the if (Build.. ) commented out, the following line is green squiggles. The warning is "This call site is reachable on 'Android 24.0' etc. I think it's just the IDE (17.5.0 P1.0), being a bit flaky because it looked to be inconsistent. So I think I've miscalled it because it can't be nullable that created the warning. Uncommenting the if (Build) and then enabling nullable doesn't change the squiggles. But adding if (OperatingSystem.IsAndroidVersionAtLeast(31)) removes the squiggles.

I'm going to presume that there is no warning in the OnCreatePreferences code because at that point the compiler can't see that far ahead to know what the PreferenceChange can call. So, since there are no warnings in the OnCreatePreferences it would appear that it is not necessary to use OperatingSystem.IsAndroidVersionAtLeast(xx) in that situation. That of course is offset by what @jonathanpeppers commented that the call to OperatingSystem.IsAndroidVersionAtLeast(xx) is technically faster anyway.

I presume there is no harm in using if (Build..) in that situation.

Would you mind confirming my assumptions, please correct me if I'm wrong. I apologise for not understanding it earlier.

I see no option for view?.Parent is View
I agree, it was just a try and obviously, it failed. Unfortunately, it also got rid of the squiggly. I also agree that I eventually found the cast suggestion. Again I thank you for the detailed explanation.

[gmck]

@jonpryor

Re your comments and the other permutations of fixing Toast's nullables.

Regarding your Toast.MakeText() question, I don't think there's a better way, unless you want to really ensure than Activity isn't null:

Toast toast = Toast.MakeText(
    Activity ?? throw new InvalidOperationException("Activity is null!"),
    Resources.GetString(Resource.String.toast_message),
    ToastLength.Long
) ?? throw new InvalidOperationException("Toast.MakeText returned null!");
toast.Show();

The following got rid of the null warnings. Would you have expected that? Is it correct?

The toast is called from inside an overridden HandleOnBackPressed of an OnBackPressedCallback, the activity can't be null because it was passed in by ctor.

Toast.MakeText(activity, message, ToastLength.Short)?.Show();

[jpobst]

I don't think there's anything left to do in this issue.