Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[LayoutBindings] LayoutBindings: 'PreserveAttribute' is obsolete warnings #8529

Merged
merged 8 commits into from
Mar 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 22 additions & 24 deletions src/Xamarin.Android.Build.Tasks/Resources/LayoutBinding.cs
Original file line number Diff line number Diff line change
@@ -1,27 +1,29 @@
using System;
#nullable enable

using System;
using System.Diagnostics.CodeAnalysis;
using Android.App;
using Android.Views;

namespace Xamarin.Android.Design
{
public delegate Java.Lang.Object OnLayoutItemNotFoundHandler (int resourceId, Type expectedViewType);
public delegate Java.Lang.Object? OnLayoutItemNotFoundHandler (int resourceId, Type expectedViewType);

abstract class LayoutBinding
{
const DynamicallyAccessedMemberTypes Constructors = DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors;

Activity boundActivity;
View boundView;
OnLayoutItemNotFoundHandler onLayoutItemNotFound;
Activity? boundActivity;
View? boundView;
OnLayoutItemNotFoundHandler? onLayoutItemNotFound;

protected LayoutBinding (Activity activity, OnLayoutItemNotFoundHandler onLayoutItemNotFound = null)
protected LayoutBinding (Activity activity, OnLayoutItemNotFoundHandler? onLayoutItemNotFound = null)
{
boundActivity = activity ?? throw new ArgumentNullException (nameof (activity));
this.onLayoutItemNotFound = onLayoutItemNotFound;
}

protected LayoutBinding (View view, OnLayoutItemNotFoundHandler onLayoutItemNotFound = null)
protected LayoutBinding (View view, OnLayoutItemNotFoundHandler? onLayoutItemNotFound = null)
{
boundView = view ?? throw new ArgumentNullException (nameof (view));
this.onLayoutItemNotFound = onLayoutItemNotFound;
Expand All @@ -38,14 +40,14 @@ protected T FindView <
if (cachedField != null)
return cachedField;

T ret;
T? ret = null;
if (boundActivity != null)
ret = boundActivity.FindViewById <T> (resourceId);
else
else if (boundView != null)
ret = boundView.FindViewById <T> (resourceId);

if (ret == null && onLayoutItemNotFound != null)
ret = (T)onLayoutItemNotFound (resourceId, typeof (T));
ret = (T?)onLayoutItemNotFound (resourceId, typeof (T));

if (ret == null)
throw new global::System.InvalidOperationException ($"View not found (Resource ID: {resourceId})");
Expand All @@ -71,16 +73,16 @@ T __FindFragment<
T
> (
int resourceId,
Func<Activity, T> finder,
ref T cachedField)
Func<Activity, T?> finder,
ref T? cachedField)
where T: Java.Lang.Object
{
if (cachedField != null)
return cachedField;

var ret = finder (EnsureActivity ());
if (ret == null && onLayoutItemNotFound != null)
ret = (T)onLayoutItemNotFound (resourceId, typeof (T));
ret = (T?)onLayoutItemNotFound (resourceId, typeof (T));

if (ret == null)
throw new InvalidOperationException ($"Fragment not found (ID: {resourceId}; Type: {typeof (T)})");
Expand All @@ -89,29 +91,25 @@ T __FindFragment<
return ret;
}
#if __ANDROID_11__
#pragma warning disable CA1422
protected T FindFragment<
[DynamicallyAccessedMembers (Constructors)]
T
> (
int resourceId,
global::Android.App.Fragment __ignoreMe,
ref T cachedField
global::Android.App.Fragment? __ignoreMe,
ref T? cachedField
)
where T: global::Android.App.Fragment
{
return __FindFragment<T> (resourceId, (activity) => activity.FragmentManager.FindFragmentById<T> (resourceId), ref cachedField);
return __FindFragment<T> (resourceId, (activity) => activity.FragmentManager?.FindFragmentById<T> (resourceId), ref cachedField);
}
#pragma warning restore CA1422
#endif // __ANDROID_11__

#if __HAVE_SUPPORT__
protected T FindFragment <T> (int resourceId, global::Android.Support.V4.App.Fragment __ignoreMe, ref T cachedField) where T: global::Android.Support.V4.App.Fragment
{
return __FindFragment<T> (resourceId, (activity) => activity.FragmentManager.FindFragmentById<T> (resourceId), ref cachedField);
}
#endif // __HAVE_SUPPORT__

#if __HAVE_ANDROIDX__
protected T FindFragment<T> (int resourceId, global::AndroidX.Fragment.App.Fragment __ignoreMe, ref T cachedField) where T: global::AndroidX.Fragment.App.Fragment
protected T FindFragment<T> (int resourceId, global::AndroidX.Fragment.App.Fragment? __ignoreMe, ref T? cachedField)
where T : global::AndroidX.Fragment.App.Fragment
{
return __FindFragment(resourceId, (activity) => {
if (activity is AndroidX.Fragment.App.FragmentActivity activity_) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,6 @@ public sealed class State
// generating code for the partial Activity class
public string BindingClassName { get; }

public bool LinkerPreserveConstructors { get; set; }

public List<string> ExtraImportNamespaces { get; } = new List <string> ();

public string AndroidFragmentType { get; }
Expand Down Expand Up @@ -155,20 +153,18 @@ protected virtual void WriteOnSetContentViewPartials (State state)
WritePartialClassOnSetContentViewPartial_Int (state);
}

public State BeginBindingFile (StreamWriter writer, string layoutResourceId, string classNamespace, string className, string androidFragmentType, bool linkerPreserveConstructors = true)
public State BeginBindingFile (StreamWriter writer, string layoutResourceId, string classNamespace, string className, string androidFragmentType)
{
var state = new State (writer, className, !String.IsNullOrWhiteSpace (classNamespace), androidFragmentType) {
LinkerPreserveConstructors = linkerPreserveConstructors
};
var state = new State (writer, className, !String.IsNullOrWhiteSpace (classNamespace), androidFragmentType);
BeginBindingFile (state, layoutResourceId, classNamespace, className);
WriteBindingConstructors (state, className, state.LinkerPreserveConstructors);
WriteBindingConstructors (state, className);
return state;
}

protected abstract void BeginBindingFile (State state, string layoutResourceId, string classNamespace, string className);
public abstract void EndBindingFile (State state);

protected abstract void WriteBindingConstructors (State state, string className, bool linkerPreserve);
protected abstract void WriteBindingConstructors (State state, string className);
protected abstract void WriteBindingViewProperty (State state, LayoutWidget widget, string resourceNamespace);
protected abstract void WriteBindingFragmentProperty (State state, LayoutWidget widget, string resourceNamespace);
protected abstract void WriteBindingMixedProperty (State state, LayoutWidget widget, string resourceNamespace);
Expand Down Expand Up @@ -287,10 +283,11 @@ protected void WriteBindingPropertyBackingField (State state, LayoutWidget widge
WriteResetLocation (state);
}

public void WriteComment (State state, string text)
public void WriteComment (State state, params string [] text)
{
EnsureArgument (state, nameof (state));
WriteLineIndent (state, $"{LineCommentString}{text}");
foreach (string line in text)
WriteLineIndent (state, $"{LineCommentString}{line}");
}

public void WriteComment (State state, ICollection<string> lines)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -157,8 +157,33 @@ void WriteClassClose (State state)
WriteLineIndent (state, "}");
}

void WriteDisableWarnings (State state)
{
state.WriteLine ("#pragma warning disable CS8981");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CS8981 is presumably because we're emitting types based on filenames? Or is that for some other reason?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is because some of the fields which we might auto generate might be all lowercase, the warning comes up as a result. for example having a property called settings because you have layout called settings.xml will raise this warning. I don't think we should be raising any warnings from generated code.

state.WriteLine ("#pragma warning disable CS1591");
}

void WriteEnableWarnings (State state)
{
state.WriteLine ("#pragma warning restore CS1591");
state.WriteLine ("#pragma warning restore CS8981");
}

void WriteAutoGeneratedComment (State state)
{
state.WriteLine (@"//------------------------------------------------------------------------------
// <auto-generated>
// This code was generated by a tool.
//
// Changes to this file may cause incorrect behavior and will be lost if
// the code is regenerated.
// </auto-generated>
//------------------------------------------------------------------------------");
}

void WriteFilePreamble (State state, string classNamespace)
{
WriteAutoGeneratedComment (state);
WriteUsings (state);
state.WriteLine ();
WriteNamespaceOpen (state, classNamespace);
Expand All @@ -172,13 +197,15 @@ void WriteNamespaceOpen (State state, string classNamespace)
state.WriteLine ($"namespace {classNamespace}");
state.WriteLine ("{");
state.IncreaseIndent ();
WriteDisableWarnings (state);
}

void WriteNamespaceClose (State state)
{
if (!state.IsInNamespace)
return;

WriteEnableWarnings (state);
state.DecreaseIndent ();
state.WriteLine ("}");
}
Expand All @@ -200,15 +227,14 @@ void WriteUsing (State state, string ns)
state.WriteLine ($"using global::{ns};");
}

protected override void WriteBindingConstructors (State state, string className, bool linkerPreserve)
protected override void WriteBindingConstructors (State state, string className)
{
WriteConstructor (state, className, "Android.App.Activity", linkerPreserve);
WriteConstructor (state, className, "Android.Views.View", linkerPreserve);
WriteConstructor (state, className, "Android.App.Activity");
WriteConstructor (state, className, "Android.Views.View");
}

void WriteConstructor (State state, string className, string clientType, bool linkerPreserve)
void WriteConstructor (State state, string className, string clientType)
{
WritePreserveAtribute (state, linkerPreserve);
WriteLineIndent (state, $"public {className} (");
state.IncreaseIndent ();
WriteLineIndent (state, $"global::{clientType} client,");
Expand All @@ -221,14 +247,6 @@ void WriteConstructor (State state, string className, string clientType, bool li
state.WriteLine ();
}

void WritePreserveAtribute (State state, bool linkerPreserve)
{
if (!linkerPreserve)
return;

WriteLineIndent (state, $"[global::Android.Runtime.PreserveAttribute (Conditional=true)]");
}

public override void EndBindingFile (State state)
{
EnsureArgument (state, nameof (state));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,10 @@ public class GenerateResourceDesignerIntermediateClass : AndroidTask

namespace %NAMESPACE% {
#pragma warning disable IDE0002
/// <summary>
/// Android Resource Designer class.
/// Exposes the Android Resource designer assembly into the project Namespace.
/// </summary>
public partial class Resource : %BASECLASS% {
}
#pragma warning restore IDE0002
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -339,6 +339,7 @@ void SuccessfulBuild_AndroidX (TestProjectInfo testInfo, bool many, bool dtb, Lo

CopyLogs (testInfo, true);
Assert.That (success, Is.True, "Build should have succeeded");
Assert.IsTrue (StringAssertEx.ContainsText (builder.LastBuildOutput, " 0 Warning(s)"), $"{builder.BuildLogFile} should have no MSBuild warnings.");

CopyGeneratedFiles (testInfo);

Expand Down Expand Up @@ -508,7 +509,8 @@ string GetBuildTarget (bool isDTB)
string[] GetBuildProperties (LocalBuilder builder, bool manyBuild, bool dtbBuild, bool referenceAndroidX, params string[] extraConstants)
{
var ret = new List <string> {
"AndroidGenerateLayoutBindings=true"
"AndroidGenerateLayoutBindings=true",
"\"NoWarn=CS0414;CA1416;CS1591;XA1005;XA4225\""
};
if (manyBuild)
ret.Add ("ForceParallelBuild=true");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,6 @@
<ProjectReference Include="..\CommonSampleLibrary\CommonSampleLibrary.NET.csproj" />
</ItemGroup>
<ItemGroup Condition=" '$(ReferenceAndroidX)' == 'True' ">
<PackageReference Include="Xamarin.AndroidX.AppCompat" Version="1.4.1.1" />
<PackageReference Include="Xamarin.AndroidX.AppCompat" Version="1.6.1.5" />
</ItemGroup>
</Project>
2 changes: 2 additions & 0 deletions tests/CodeBehind/BuildTests/MainMergeActivity.cs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ protected override void OnCreate (Bundle savedInstanceState)

#if NOT_CONFLICTING_FRAGMENT
CommonSampleLibrary.LogFragment log2 = secondary_log_fragment;
#elif __HAVE_ANDROIDX__
global::AndroidX.Fragment.App.Fragment log2 = secondary_log_fragment;
#else
global::Android.App.Fragment log2 = secondary_log_fragment;
#endif
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
<?xml version="1.0" encoding="utf-8"?>
<manifest xmlns:android="http://schemas.android.com/apk/res/android" android:versionCode="1" android:versionName="1.0" package="com.xamarin.CodeBehindBuildTests">
<uses-sdk android:minSdkVersion="21" />
<application android:allowBackup="true" android:icon="@mipmap/icon" android:label="@string/app_name">
</application>
</manifest>
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,10 @@
<AssemblyName>CommonSampleLibrary</AssemblyName>
<AppendTargetFrameworkToOutputPath>false</AppendTargetFrameworkToOutputPath>
</PropertyGroup>
<PropertyGroup>
<DefineConstants Condition=" '$(ExtraConstants)' != '' ">$(DefineConstants);$(ExtraConstants)</DefineConstants>
</PropertyGroup>
<ItemGroup Condition=" '$(ReferenceAndroidX)' == 'True' ">
<PackageReference Include="Xamarin.AndroidX.AppCompat" Version="1.6.1.5" />
</ItemGroup>
</Project>
12 changes: 10 additions & 2 deletions tests/CodeBehind/CommonSampleLibrary/Logger/LogFragment.cs
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,21 @@
using Android.Views;
using Android.Widget;
using Android.Graphics;
using Android.Util;

namespace CommonSampleLibrary
{
#pragma warning disable CA1422
/**
* Simple fraggment which contains a LogView and uses is to output log data it receives
* through the LogNode interface.
*/
public class LogFragment : Fragment
public class LogFragment :
#if __HAVE_ANDROIDX__
AndroidX.Fragment.App.Fragment
#else
Fragment
#endif
{
LogView mLogView;
ScrollView mScrollView;
Expand Down Expand Up @@ -60,7 +67,7 @@ public View InflateViews ()

mLogView.Gravity = GravityFlags.Bottom;
mLogView.SetTextAppearance (Activity, Android.Resource.Style.TextAppearanceMedium);

mScrollView.AddView (mLogView);
return mScrollView;
}
Expand All @@ -77,5 +84,6 @@ public LogView LogView {
get { return mLogView; }
}
}
#pragma warning restore CA1422
}