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

feat: Add flag to enable analyzers #336

Closed
wants to merge 11 commits into from
Closed
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
4 changes: 4 additions & 0 deletions src/Uno.Templates/content/unoapp/.globalconfig
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
is_global = true

# IDE0055: Fix formatting
dotnet_diagnostic.IDE0055.severity = warning
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd rather not add this file. You should be able to set the diagnostic in the .editorconfig

Copy link
Member Author

Choose a reason for hiding this comment

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

.globalconfig is now considered the "modern"/"encouraged" alternative of .editorconfig. I'd definitely use .globalconfigs for setting severities instead of .editorconfigs. It also has a "slight" performance improvement on Roslyn side since globalconfigs are evaluated per project rather than per file.

Copy link
Contributor

Choose a reason for hiding this comment

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

Unless we have something compelling from Microsoft for why we should move away from .editorconfig I'm hesitant to allow yet another file in. Not to mention what you are doing here is already supported in .editorconfig, and we already configure diagnostics in the .editorconfig. We don't need to make it harder on developers by giving them 2 files to maintain.

Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,10 @@
"longName": "dependency-injection",
"shortName": "di"
},
"analyzers": {
"longName": "analyzers",
"shortName": "analyzers"
},
"configuration": {
"longName": "configuration",
"shortName": "config"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -406,6 +406,13 @@
"type": "parameter",
"datatype": "bool"
},
"analyzers": {
"displayName": "Analyzers",
"description": "Enable C# analyzers",
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's give this a better description... what do I get with these analyzers? Technically even without this Analyzers are enabled in the project... it's a specific subset that we're enabling here...

"type": "parameter",
"datatype": "bool",
"defaultValue": "true"
},
"configuration": {
"displayName": "Configuration",
"description": "Load configuration information from appsettings.json",
Expand Down
11 changes: 11 additions & 0 deletions src/Uno.Templates/content/unoapp/Directory.Build.props
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,17 @@
<AndroidMaterialVersion Condition=" '$(AndroidMaterialVersion)' == '' ">1.9.0.2</AndroidMaterialVersion>
<AndroidXNavigationVersion Condition=" '$(AndroidXNavigationVersion)' == '' ">2.6.0.1</AndroidXNavigationVersion>
<!--#endif-->
<!--#if (analyzers)-->

<EnforceCodeStyleInBuild>true</EnforceCodeStyleInBuild>
<EnableNETAnalyzers>true</EnableNETAnalyzers>
<AnalysisMode>Recommended</AnalysisMode>
<TreatWarningsAsErrors>true</TreatWarningsAsErrors>
Copy link
Member Author

Choose a reason for hiding this comment

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

We probably should remove this, and instead pass it here:

& dotnet build UnoApp1.sln "/bl:$(build.artifactstagingdirectory)\$(Agent.JobName).binlog"


<!-- CA1010: Type 'MainPage' directly or indirectly inherits 'IEnumerable' without implementing 'IEnumerable<T>' -->
<!-- CA1848: For improved performance, use the LoggerMessage delegates -->
Copy link
Member Author

Choose a reason for hiding this comment

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

We will need to look into this.

<NoWarn>$(NoWarn);CA1010;CA1848</NoWarn>
<!--#endif-->
</PropertyGroup>

<Choose>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ public EmbeddedControl()

//+:cnd:noEmit
#if (!useMvvmOrMvux)
private int count=0;
private int count;
public void CounterClicked(object sender, EventArgs e)
{
CounterButton.Text = ++count switch
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,16 @@ namespace MyExtensionsApp._1;

public class UnoImageConverter : IValueConverter
{
public object Convert(object value, Type targetType, object parameter, CultureInfo culture)
public object? Convert(object? value, Type targetType, object? parameter, CultureInfo culture)
{
#if ANDROID
return (value + "").Replace('/','_').Replace('\\','_');
return (value + "").Replace('/', '_').Replace('\\', '_');
#else
return value;
#endif
}

public object ConvertBack(object value, Type targetType, object parameter, CultureInfo culture)
public object? ConvertBack(object? value, Type targetType, object? parameter, CultureInfo culture)
{
throw new NotImplementedException();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ private static IEnumerable<WeatherForecast> GetForecast(ILoggerFactory loggerFac
{
logger.LogInformation("Weather forecast for {Date} is a {Summary} {TemperatureC}°C", x.Date, x.Summary, x.TemperatureC);
return x;
})
.ToArray();
});
}
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
//+:cnd:noEmit
#if (useSerilog)
using System.Globalization;
#endif
#if (useHttp)
using System.Text.Json.Serialization.Metadata;
#endif
Expand All @@ -16,8 +19,8 @@
{
#if (useSerilog)
Log.Logger = new LoggerConfiguration()
.WriteTo.Console()
.WriteTo.File(Path.Combine("App_Data", "Logs", "log.txt"))
.WriteTo.Console(formatProvider: CultureInfo.InvariantCulture)
.WriteTo.File(Path.Combine("App_Data", "Logs", "log.txt"), formatProvider: CultureInfo.InvariantCulture)
.CreateLogger();
#endif
var builder = WebApplication.CreateBuilder(args);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,12 @@

<PropertyGroup>
<TargetFramework>$baseTargetFramework$</TargetFramework>
<!--#if (analyzers)-->

<!-- CA1707: Remove the underscores from type name -->
<NoWarn>$(NoWarn);CA1707</NoWarn>
<!--#endif-->

</PropertyGroup>

<ItemGroup>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
//-:cnd:noEmit
using System.Globalization;

namespace MyExtensionsApp._1;

//+:cnd:noEmit
Expand Down Expand Up @@ -123,7 +125,7 @@ protected async override void OnLaunched(LaunchActivatedEventArgs args)
credentials ??= new Dictionary<string, string>();
credentials[TokenCacheExtensions.AccessTokenKey] = "SampleToken";
credentials[TokenCacheExtensions.RefreshTokenKey] = "RefreshToken";
credentials["Expiry"] = DateTime.Now.AddMinutes(5).ToString("g");
credentials["Expiry"] = DateTime.Now.AddMinutes(5).ToString("g", DateTimeFormatInfo.InvariantInfo);
Copy link
Member Author

Choose a reason for hiding this comment

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

NOTE: This is a behavioral change. When using a null provider, the used default will be CurrentInfo which doesn't seem suitable here.

https://github.com/dotnet/runtime/blob/4001e074ee648506d64277729a7cce089a4996e2/src/libraries/System.Private.CoreLib/src/System/Globalization/DateTimeFormatInfo.cs#L314

return ValueTask.FromResult<IDictionary<string, string>?>(credentials);
}

Expand All @@ -142,7 +144,7 @@ protected async override void OnLaunched(LaunchActivatedEventArgs args)
// Return IDictionary containing any tokens used by service calls or in the app
tokenDictionary ??= new Dictionary<string, string>();
tokenDictionary[TokenCacheExtensions.AccessTokenKey] = "NewSampleToken";
tokenDictionary["Expiry"] = DateTime.Now.AddMinutes(5).ToString("g");
tokenDictionary["Expiry"] = DateTime.Now.AddMinutes(5).ToString("g", DateTimeFormatInfo.InvariantInfo);
return ValueTask.FromResult<IDictionary<string, string>?>(tokenDictionary);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,13 @@ public AppResources()
#if useToolkit
// Load Uno.UI.Toolkit and Material Resources
this.Build(r => r.Merged(
new MaterialToolkitTheme(
new MaterialToolkitTheme(
new Styles.ColorPaletteOverride(),
new Styles.MaterialFontsOverride())));
#else
// Load Uno.UI.Toolkit and Material Resources
this.Build(r => r.Merged(
new MaterialTheme(
new MaterialTheme(
new Styles.ColorPaletteOverride(),
new Styles.MaterialFontsOverride())));
#endif
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
//+:cnd:noEmit
namespace MyExtensionsApp._1.Infrastructure;

internal class DebugHttpHandler : DelegatingHandler
internal sealed class DebugHttpHandler : DelegatingHandler
{
#if (useLogging)
private readonly ILogger _logger;
Expand Down Expand Up @@ -34,13 +34,13 @@ protected async override Task<HttpResponseMessage> SendAsync(
{
_logger.LogDebugMessage($"{request.RequestUri} ({request.Method})");
}

foreach ((var key, var values) in request.Headers.ToDictionary(x => x.Key, x => string.Join(", ", x.Value)))
{
_logger.LogDebugMessage($"{key}: {values}");
}

var content = request.Content is not null ? await request.Content.ReadAsStringAsync() : null;
var content = request.Content is not null ? await request.Content.ReadAsStringAsync(cancellationToken) : null;
if (!string.IsNullOrEmpty(content))
{
_logger.LogDebugMessage(content);
Expand All @@ -57,7 +57,7 @@ protected async override Task<HttpResponseMessage> SendAsync(
Console.Error.WriteLine($" {key}: {values}");
}

var content = request.Content is not null ? await request.Content.ReadAsStringAsync() : null;
var content = request.Content is not null ? await request.Content.ReadAsStringAsync(cancellationToken) : null;
if (!string.IsNullOrEmpty(content))
{
Console.Error.WriteLine(content);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ public partial record LoginModel(IDispatcher Dispatcher, INavigator Navigator, I

//+:cnd:noEmit
#if useCustomAuthentication
public IState<string> Username => State<string>.Value(this, () => string.Empty);
public IState<string> Username => State<string>.Value(this, () => string.Empty);

public IState<string> Password => State<string>.Value(this, () => string.Empty);
#endif
Expand All @@ -25,8 +25,7 @@ public async ValueTask Login(CancellationToken token)
//-:cnd:noEmit
if (success)
{
await Navigator.NavigateViewModelAsync<MainModel>(this, qualifier: Qualifiers.ClearBackStack);
await Navigator.NavigateViewModelAsync<MainModel>(this, qualifier: Qualifiers.ClearBackStack, cancellation: token);
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ public WeatherCache(IApiClient api, ISerializer serializer)
}
#endif

private bool IsConnected => NetworkInformation.GetInternetConnectionProfile().GetNetworkConnectivityLevel() == NetworkConnectivityLevel.InternetAccess;
private static bool IsConnected => NetworkInformation.GetInternetConnectionProfile().GetNetworkConnectivityLevel() == NetworkConnectivityLevel.InternetAccess;

public async ValueTask<IImmutableList<WeatherForecast>> GetForecast(CancellationToken token)
{
Expand All @@ -38,7 +38,7 @@ public async ValueTask<IImmutableList<WeatherForecast>> GetForecast(Cancellation
#if (useLogging)
_logger.LogWarning("App is offline and cannot connect to the API.");
#endif
throw new Exception("No internet connection");
throw new InvalidOperationException("No internet connection");
Copy link
Contributor

Choose a reason for hiding this comment

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

Thinking maybe change this to:

Suggested change
throw new InvalidOperationException("No internet connection");
throw new NetworkException(NetworkExceptionCategory.Offline, 0, "No internet connection");

}

var response = await _api.GetWeather(token);
Expand All @@ -62,10 +62,10 @@ public async ValueTask<IImmutableList<WeatherForecast>> GetForecast(Cancellation
}
}

private async ValueTask<StorageFile> GetFile(CreationCollisionOption option) =>
private static async ValueTask<StorageFile> GetFile(CreationCollisionOption option) =>
await ApplicationData.Current.TemporaryFolder.CreateFileAsync("weather.json", option);

private async ValueTask<string?> GetCachedWeather()
private static async ValueTask<string?> GetCachedWeather()
{
var file = await GetFile(CreationCollisionOption.OpenIfExists);
var properties = await file.GetBasicPropertiesAsync();
Expand All @@ -84,6 +84,6 @@ private async ValueTask Save(IImmutableList<WeatherForecast> weather, Cancellati
{
var weatherText = _serializer.ToString(weather);
var file = await GetFile(CreationCollisionOption.ReplaceExisting);
await File.WriteAllTextAsync(file.Path, weatherText);
await File.WriteAllTextAsync(file.Path, weatherText, token);
}
}
Loading