Resolved more warnings, and marked more warning types as errors (#16990)

* Fix warnings SA1111, SA1028, SA1500, IDE1270  in Umbraco.Web.Website, and updated rules.

* Remove warnings: IDE0270: Null check can be simplified

* More SqlServer project warnings resolved

* CS0105 namespace appeared already

* Suppress warning until implementation:

#pragma warning disable CS0162 // Unreachable code detected
#pragma warning disable CS0618 // Type or member is obsolete

CS0162 remove unreachable code
SA1028 remove trailing whitespace
SA1106 no empty statements
CS1570 malformed XML
CS1572 corrected xml parameter
CS1573 param tag added
IDE0007 var not explicit
IDE0008 explicit not var
IDE0057 simplify substring
IDE0074 compound assignment
CA1825 array.empty

Down to 3479 warnings

src/Umbraco.Cms.Api.Management/Controllers/Indexer/RebuildIndexerController.cs

@@ -1,4 +1,4 @@
-using Asp.Versioning;
+using Asp.Versioning;
 using Examine;
 using Microsoft.AspNetCore.Http;
 using Microsoft.AspNetCore.Mvc;
@@ -38,7 +38,7 @@ public RebuildIndexerController(
  [ProducesResponseType(StatusCodes.Status200OK)]
  public async Task<IActionResult> Rebuild(CancellationToken cancellationToken, string indexName)
  {
- if (!_examineManager.TryGetIndex(indexName, out var index))
+ if (!_examineManager.TryGetIndex(indexName, out IIndex? index))
  {
  var invalidModelProblem = new ProblemDetails
  {

src/Umbraco.Cms.Api.Management/Controllers/Security/BackOfficeController.cs

@@ -1,4 +1,4 @@
-using System.Security.Claims;
+using System.Security.Claims;
 using Asp.Versioning;
 using Microsoft.AspNetCore;
 using Microsoft.AspNetCore.Authentication;
@@ -217,7 +217,7 @@ public async Task<IActionResult> LinkLoginKey(string provider)
  /// <summary>
  /// Called when a user links an external login provider in the back office
  /// </summary>
- /// <param name="provider"></param>
+ /// <param name="requestModel"></param>
  /// <returns></returns>
  // This method is marked as AllowAnonymous and protected with a secret (linkKey) inside the model for the following reasons
  // - when a js client uses the fetch api (or old ajax requests) they can send a bearer token

src/Umbraco.Cms.Api.Management/Factories/MediaTypeEditingPresentationFactory.cs

@@ -1,5 +1,4 @@
-using Umbraco.Cms.Api.Management.ViewModels.MediaType;
-using Umbraco.Cms.Core.Models;
+using Umbraco.Cms.Api.Management.ViewModels.MediaType;
 using Umbraco.Cms.Core.Models;
 using Umbraco.Cms.Core.Models.ContentTypeEditing;
 using Umbraco.Cms.Core.Services;

src/Umbraco.Cms.Persistence.EFCore/Locking/SqliteEFCoreDistributedLockingMechanism.cs

@@ -115,12 +115,7 @@ public override string ToString()
  // Mostly no-op just check that we didn't end up ReadUncommitted for real.
  private void ObtainReadLock()
  {
- IEfCoreScope<T>? efCoreScope = _parent._efCoreScopeAccessor.Value.AmbientScope;
-
- if (efCoreScope is null)
- {
- throw new PanicException("No current ambient scope");
- }
+ IEfCoreScope<T>? efCoreScope = _parent._efCoreScopeAccessor.Value.AmbientScope ?? throw new PanicException("No current ambient scope");
 
  efCoreScope.ExecuteWithContextAsync<Task>(async database =>
  {
@@ -136,12 +131,7 @@ private void ObtainReadLock()
  // lock occurs for entire database as opposed to row/table.
  private void ObtainWriteLock()
  {
- IEfCoreScope<T>? efCoreScope = _parent._efCoreScopeAccessor.Value.AmbientScope;
-
- if (efCoreScope is null)
- {
- throw new PanicException("No ambient scope");
- }
+ IEfCoreScope<T>? efCoreScope = _parent._efCoreScopeAccessor.Value.AmbientScope ?? throw new PanicException("No ambient scope");
 
  efCoreScope.ExecuteWithContextAsync<Task>(async database =>
  {

src/Umbraco.Cms.Persistence.SqlServer/LocalDb.cs

@@ -420,7 +420,13 @@ public bool DatabaseExists(string databaseName)
  /// </remarks>
  public bool CreateDatabase(string databaseName, string filesPath)
  {
- GetDatabaseFiles(databaseName, filesPath, out var logName, out _, out _, out var mdfFilename,
+ GetDatabaseFiles(
+ databaseName,
+ filesPath,
+ out var logName,
+ out _,
+ out _,
+ out var mdfFilename,
  out var ldfFilename);
 
  using (var conn = new SqlConnection(_masterCstr))
@@ -463,7 +469,9 @@ public bool DropDatabase(string databaseName)
  {
  conn.Open();
 
- SetCommand(cmd, @"
+ SetCommand(
+ cmd,
+ @"
  SELECT name, filename FROM master.dbo.sysdatabases WHERE ('[' + name + ']' = @0 OR name = @0)",
  databaseName);
 
@@ -554,11 +562,7 @@ public int DropDatabases(bool staleOnly = false)
  {
  conn.Open();
 
- var mdf = GetDatabase(cmd, databaseName);
- if (mdf == null)
- {
- throw new InvalidOperationException("Database does not exist.");
- }
+ var mdf = GetDatabase(cmd, databaseName) ?? throw new InvalidOperationException("Database does not exist.");
 
  DetachDatabase(cmd, databaseName);
 
@@ -597,9 +601,12 @@ public void AttachDatabase(string databaseName, string filesPath)
  /// <param name="ldfName">The LDF logical name.</param>
  /// <param name="mdfFilename">The MDF filename.</param>
  /// <param name="ldfFilename">The LDF filename.</param>
- public void GetFilenames(string databaseName,
- out string? mdfName, out string? ldfName,
- out string? mdfFilename, out string? ldfFilename)
+ public void GetFilenames(
+ string databaseName,
+ out string? mdfName,
+ out string? ldfName,
+ out string? mdfFilename,
+ out string? ldfFilename)
  {
  using (var conn = new SqlConnection(_masterCstr))
  using (SqlCommand? cmd = conn.CreateCommand())
@@ -621,7 +628,9 @@ public void KillConnections(string databaseName)
  {
  conn.Open();
 
- SetCommand(cmd, @"
+ SetCommand(
+ cmd,
+ @"
  DECLARE @sql VARCHAR(MAX);
  SELECT @sql = COALESCE(@sql,'') + 'kill ' + CONVERT(VARCHAR, SPId) + ';'
  FROM master.sys.sysprocesses
@@ -640,7 +649,9 @@ FROM master.sys.sysprocesses
  /// <returns>The full filename of the MDF file, if the database exists, otherwise null.</returns>
  private static string? GetDatabase(SqlCommand cmd, string databaseName)
  {
- SetCommand(cmd, @"
+ SetCommand(
+ cmd,
+ @"
  SELECT name, filename FROM master.dbo.sysdatabases WHERE ('[' + name + ']' = @0 OR name = @0)",
  databaseName);
 
@@ -707,7 +718,7 @@ private static void ExecuteDropDatabase(SqlCommand cmd, string databaseName, str
  File.Delete(mdf);
  }
 
- ldf = ldf ?? GetLogFilename(mdf);
+ ldf ??= GetLogFilename(mdf);
  if (File.Exists(ldf))
  {
  File.Delete(ldf);
@@ -726,7 +737,7 @@ private static string GetLogFilename(string mdfFilename)
  throw new ArgumentException("Not a valid MDF filename (no .mdf extension).", nameof(mdfFilename));
  }
 
- return mdfFilename.Substring(0, mdfFilename.Length - ".mdf".Length) + "_log.ldf";
+ return mdfFilename[..^".mdf".Length] + "_log.ldf";
  }
 
  /// <summary>
@@ -743,7 +754,9 @@ private static void DetachDatabase(SqlCommand cmd, string databaseName)
 
  var unused1 = cmd.ExecuteNonQuery();
 
- SetCommand(cmd, @"
+ SetCommand(
+ cmd,
+ @"
  EXEC sp_detach_db @dbname=@0",
  databaseName);
 
@@ -758,8 +771,14 @@ private static void DetachDatabase(SqlCommand cmd, string databaseName)
  /// <param name="filesPath">The directory containing database files.</param>
  private static void AttachDatabase(SqlCommand cmd, string databaseName, string filesPath)
  {
- GetDatabaseFiles(databaseName, filesPath,
- out var logName, out _, out _, out var mdfFilename, out var ldfFilename);
+ GetDatabaseFiles(
+ databaseName,
+ filesPath,
+ out var logName,
+ out _,
+ out _,
+ out var mdfFilename,
+ out var ldfFilename);
 
  // cannot use parameters on CREATE DATABASE
  // ie "CREATE DATABASE @0 ..." does not work
@@ -802,13 +821,19 @@ private static void SetCommand(SqlCommand cmd, string sql, params object[] args)
  /// <param name="ldfName">The LDF logical name.</param>
  /// <param name="mdfFilename">The MDF filename.</param>
  /// <param name="ldfFilename">The LDF filename.</param>
- private void GetFilenames(SqlCommand cmd, string databaseName,
- out string? mdfName, out string? ldfName,
- out string? mdfFilename, out string? ldfFilename)
+ private void GetFilenames(
+ SqlCommand cmd,
+ string databaseName,
+ out string? mdfName,
+ out string? ldfName,
+ out string? mdfFilename,
+ out string? ldfFilename)
  {
  mdfName = ldfName = mdfFilename = ldfFilename = null;
 
- SetCommand(cmd, @"
+ SetCommand(
+ cmd,
+ @"
  SELECT DB_NAME(database_id), type_desc, name, physical_name
  FROM master.sys.master_files
  WHERE database_id=DB_ID(@0)",
@@ -854,21 +879,32 @@ FROM master.sys.master_files
  /// <paramref name="delete" />.
  /// Extensions are used eg to copy MyDatabase.mdf to MyDatabase.mdf.temp.
  /// </remarks>
- public void CopyDatabaseFiles(string databaseName, string filesPath,
- string? targetDatabaseName = null, string? targetFilesPath = null,
- string? sourceExtension = null, string? targetExtension = null,
- bool overwrite = false, bool delete = false)
+ public void CopyDatabaseFiles(
+ string databaseName,
+ string filesPath,
+ string? targetDatabaseName = null,
+ string? targetFilesPath = null,
+ string? sourceExtension = null,
+ string? targetExtension = null,
+ bool overwrite = false,
+ bool delete = false)
  {
  var nop = (targetFilesPath == null || targetFilesPath == filesPath)
  && (targetDatabaseName == null || targetDatabaseName == databaseName)
- && (sourceExtension == null && targetExtension == null || sourceExtension == targetExtension);
+ && ((sourceExtension == null && targetExtension == null) || sourceExtension == targetExtension);
  if (nop && delete == false)
  {
  return;
  }
 
- GetDatabaseFiles(databaseName, filesPath,
- out _, out _, out _, out var mdfFilename, out var ldfFilename);
+ GetDatabaseFiles(
+ databaseName,
+ filesPath,
+ out _,
+ out _,
+ out _,
+ out var mdfFilename,
+ out var ldfFilename);
 
  if (sourceExtension != null)
  {
@@ -892,8 +928,14 @@ public void CopyDatabaseFiles(string databaseName, string filesPath,
  else
  {
  // copy or copy+delete ie move
- GetDatabaseFiles(targetDatabaseName ?? databaseName, targetFilesPath ?? filesPath,
- out _, out _, out _, out var targetMdfFilename, out var targetLdfFilename);
+ GetDatabaseFiles(
+ targetDatabaseName ?? databaseName,
+ targetFilesPath ?? filesPath,
+ out _,
+ out _,
+ out _,
+ out var targetMdfFilename,
+ out var targetLdfFilename);
 
  if (targetExtension != null)
  {
@@ -936,8 +978,14 @@ public void CopyDatabaseFiles(string databaseName, string filesPath,
  /// </remarks>
  public bool DatabaseFilesExist(string databaseName, string filesPath, string? extension = null)
  {
- GetDatabaseFiles(databaseName, filesPath,
- out _, out _, out _, out var mdfFilename, out var ldfFilename);
+ GetDatabaseFiles(
+ databaseName,
+ filesPath,
+ out _,
+ out _,
+ out _,
+ out var mdfFilename,
+ out var ldfFilename);
 
  if (extension != null)
  {
@@ -958,10 +1006,14 @@ public bool DatabaseFilesExist(string databaseName, string filesPath, string? ex
  /// <param name="baseLogFilename">The base log filename (the LDF filename without the .ldf extension).</param>
  /// <param name="mdfFilename">The MDF filename.</param>
  /// <param name="ldfFilename">The LDF filename.</param>
- private static void GetDatabaseFiles(string databaseName, string filesPath,
+ private static void GetDatabaseFiles(
+ string databaseName,
+ string filesPath,
  out string logName,
- out string baseFilename, out string baseLogFilename,
- out string mdfFilename, out string ldfFilename)
+ out string baseFilename,
+ out string baseLogFilename,
+ out string mdfFilename,
+ out string ldfFilename)
  {
  logName = databaseName + "_log";
  baseFilename = Path.Combine(filesPath, databaseName);

src/Umbraco.Core/Logging/DisposableTimer.cs

@@ -44,7 +44,7 @@ internal DisposableTimer(
  _endMessageArgs = endMessageArgs;
  _failMessageArgs = failMessageArgs;
  _thresholdMilliseconds = thresholdMilliseconds < 0 ? 0 : thresholdMilliseconds;
- _timingId = Guid.NewGuid().ToString("N").Substring(0, 7); // keep it short-ish
+ _timingId = Guid.NewGuid().ToString("N")[..7]; // keep it short-ish
 
  if (thresholdMilliseconds == 0)
  {
@@ -63,7 +63,7 @@ internal DisposableTimer(
  var args = new object[startMessageArgs.Length + 1];
  startMessageArgs.CopyTo(args, 0);
  args[^1] = _timingId;
- 
+
  if (_logger.IsEnabled(Microsoft.Extensions.Logging.LogLevel.Debug))
  {
  logger.LogDebug(startMessage + " [Timing {TimingId}]", args);

src/Umbraco.Core/Models/Membership/UserGroup.cs

@@ -58,8 +58,6 @@ public UserGroup(IShortStringHelper shortStringHelper)
  /// <param name="userCount"></param>
  /// <param name="alias"></param>
  /// <param name="name"></param>
- /// <param name="permissions"></param>
- /// <param name="granularPermissions"></param>
  /// <param name="icon"></param>
  /// <param name="shortStringHelper"></param>
  public UserGroup(

src/Umbraco.Core/Persistence/Repositories/ITrackedReferencesRepository.cs

@@ -54,7 +54,7 @@ public interface ITrackedReferencesRepository
  /// Gets a page of items which are in relation with the current item.
  /// Basically, shows the items which depend on the current item.
  /// </summary>
- /// <param name="id">The identifier of the entity to retrieve relations for.</param>
+ /// <param name="key">The identifier of the entity to retrieve relations for.</param>
  /// <param name="skip">The amount of items to skip.</param>
  /// <param name="take">The amount of items to take.</param>
  /// <param name="filterMustBeIsDependency">
@@ -81,7 +81,7 @@ IEnumerable<RelationItemModel> GetPagedRelationsForItem(
  /// <summary>
  /// Gets a page of items used in any kind of relation from selected integer ids.
  /// </summary>
- /// <param name="ids">The identifiers of the entities to check for relations.</param>
+ /// <param name="keys">The identifiers of the entities to check for relations.</param>
  /// <param name="skip">The amount of items to skip.</param>
  /// <param name="take">The amount of items to take.</param>
  /// <param name="filterMustBeIsDependency">

src/Umbraco.Core/PropertyEditors/JsonPropertyIndexValueFactoryBase.cs

@@ -2,7 +2,6 @@
 using Umbraco.Cms.Core.Configuration.Models;
 using Umbraco.Cms.Core.Models;
 using Umbraco.Cms.Core.Serialization;
-using Umbraco.Cms.Core.Serialization;
 using Umbraco.Extensions;
 
 namespace Umbraco.Cms.Core.PropertyEditors;

src/Umbraco.Core/Routing/ContentFinderByUrlAndTemplate.cs

@@ -77,8 +77,8 @@ public override Task<bool> TryFindContent(IPublishedRequestBuilder frequest)
 
  // look for template in last position
  var pos = path.LastIndexOf('/');
- var templateAlias = path.Substring(pos + 1);
- path = pos == 0 ? "/" : path.Substring(0, pos);;
+ var templateAlias = path[(pos + 1)..];
+ path = pos == 0 ? "/" : path[..pos];
 
  ITemplate? template = _fileService.GetTemplate(templateAlias);
 

src/Umbraco.Core/Routing/DefaultUrlProvider.cs

@@ -98,7 +98,7 @@ public virtual IEnumerable<UrlInfo> GetOtherUrls(int id, Uri current)
 
  // need to strip off the leading ID for the route if it exists (occurs if the route is for a node with a domain assigned)
  var pos = route.IndexOf('/');
- var path = pos == 0 ? route : route.Substring(pos);
+ var path = pos == 0 ? route : route[pos..];
 
  var uri = new Uri(CombinePaths(d.Uri.GetLeftPart(UriPartial.Path), path));
  uri = _uriUtility.UriFromUmbraco(uri, _requestSettings);