Fixed all compiler warnings (#675)

* Fixed all compiler warnings

Also corrected Equals implementation and set builds to fail on compiler warnings in release mode.

* Suppress warning-as-error "CS1591: Missing XML comment for publicly visible type or member" because it is a massive amount of work to fix all of those.

* Fix CS1570: XML comment has badly formed XML

* Fix CS1584: XML comment has syntactically incorrect cref attribute

* Fix CS1587: XML comment is not placed on a valid language element

* Fix CS1574: XML comment has cref attribute that could not be resolved

* Fix CS1572: XML comment has a param tag, but there is no parameter by that name

* CS1711: XML comment has a typeparam tag, but there is no type parameter by that name

* CS1573: Parameter has no matching param tag in the XML comment (but other parameters do)

* Fix CS1723: XML comment has cref attribute that refers to a type parameter

* Fix CS1734: XML comment has a paramref tag, but there is no parameter by that name

* Updated fixed package versions with placeholders in project files

* Disabled building benchmarks project in Release mode (was already disabled for Debug)

The project type GUID was changed by VS 2019 v16.4.3 automatically. According to https://stackoverflow.com/questions/47312163/dotnet-sln-add-wrong-project-type-guid, this seems like a good thing.

* Fix for NU5125: The 'licenseUrl' element will be deprecated.

* Update HasManyAttribute.cs

* Update HasOneAttribute.cs

* docs: add compiler warning instructions

* docs: spacing

Co-authored-by: Maurits Moeys <maurei@users.noreply.github.com>

Author: Bart Koelman

Directory.Build.props

@@ -13,6 +13,11 @@
     <TuplesVersion>4.5.0</TuplesVersion>
   </PropertyGroup>
 
+  <PropertyGroup Condition="'$(Configuration)'=='Release'">
+    <NoWarn>$(NoWarn);1591</NoWarn>
+    <TreatWarningsAsErrors>true</TreatWarningsAsErrors>
+  </PropertyGroup>
+
   <!-- Test Project Dependencies -->
   <PropertyGroup>
     <TestSdkVersion>16.3.0</TestSdkVersion>

JsonApiDotnetCore.sln

@@ -108,7 +108,6 @@ Global
 		{DF0FCFB2-CB12-44BA-BBB5-1BE0BCFCD14C}.Debug|x86.ActiveCfg = Debug|Any CPU
 		{DF0FCFB2-CB12-44BA-BBB5-1BE0BCFCD14C}.Debug|x86.Build.0 = Debug|Any CPU
 		{DF0FCFB2-CB12-44BA-BBB5-1BE0BCFCD14C}.Release|Any CPU.ActiveCfg = Release|Any CPU
-		{DF0FCFB2-CB12-44BA-BBB5-1BE0BCFCD14C}.Release|Any CPU.Build.0 = Release|Any CPU
 		{DF0FCFB2-CB12-44BA-BBB5-1BE0BCFCD14C}.Release|x64.ActiveCfg = Release|Any CPU
 		{DF0FCFB2-CB12-44BA-BBB5-1BE0BCFCD14C}.Release|x64.Build.0 = Release|Any CPU
 		{DF0FCFB2-CB12-44BA-BBB5-1BE0BCFCD14C}.Release|x86.ActiveCfg = Release|Any CPU

README.md

@@ -109,6 +109,10 @@ Sometimes the compiled files can be dirty / corrupt from other branches / failed
 dotnet clean
 ```
 
+#### Compiler warnings
+The `Release` build configuration is set to fail on warnings. That means when submitting a PR there shouldn't be any compiler warnings because the CI build it set to `Release`.
+
+
 ## Compatibility
 
 A lot of changes were introduced in v4.0.0, the following chart should help you with compatibility issues between .NET Core versions

src/JsonApiDotNetCore/Builders/ResourceGraphBuilder.cs

@@ -91,9 +91,9 @@ protected virtual List<AttrAttribute> GetAttributes(Type entityType)
 
             foreach (var prop in properties)
             {
-                /// todo: investigate why this is added in the exposed attributes list
-                /// because it is not really defined attribute considered from the json:api
-                /// spec point of view.
+                // todo: investigate why this is added in the exposed attributes list
+                // because it is not really defined attribute considered from the json:api
+                // spec point of view.
                 if (prop.Name == nameof(Identifiable.Id))
                 {
                     var idAttr = new AttrAttribute()

src/JsonApiDotNetCore/Configuration/ILinksConfiguration.cs

@@ -1,3 +1,4 @@
+using JsonApiDotNetCore.Models;
 using JsonApiDotNetCore.Models.Links;
 
 namespace JsonApiDotNetCore.Configuration
@@ -68,4 +69,4 @@ public interface ILinksConfiguration
         Link RelationshipLinks { get; }
 
     }
-}
+}

src/JsonApiDotNetCore/Controllers/JsonApiController.cs

@@ -74,7 +74,6 @@ public class JsonApiController<T> : JsonApiController<T, int> where T : class, I
         /// Base constructor with int as default
         /// </summary>
         /// <param name="jsonApiOptions"></param>
-        /// <param name="resourceGraph"></param>
         /// <param name="resourceService"></param>
         /// <param name="loggerFactory"></param>
         public JsonApiController(

src/JsonApiDotNetCore/Data/DefaultResourceRepository.cs

@@ -89,11 +89,11 @@ public virtual async Task<TResource> CreateAsync(TResource entity)
                 object trackedRelationshipValue = GetTrackedRelationshipValue(relationshipAttr, entity, out bool relationshipWasAlreadyTracked);
                 LoadInverseRelationships(trackedRelationshipValue, relationshipAttr);
                 if (relationshipWasAlreadyTracked || relationshipAttr is HasManyThroughAttribute)
-                    /// We only need to reassign the relationship value to the to-be-added
-                    /// entity when we're using a different instance of the relationship (because this different one
-                    /// was already tracked) than the one assigned to the to-be-created entity.
-                    /// Alternatively, even if we don't have to reassign anything because of already tracked 
-                    /// entities, we still need to assign the "through" entities in the case of many-to-many.
+                    // We only need to reassign the relationship value to the to-be-added
+                    // entity when we're using a different instance of the relationship (because this different one
+                    // was already tracked) than the one assigned to the to-be-created entity.
+                    // Alternatively, even if we don't have to reassign anything because of already tracked 
+                    // entities, we still need to assign the "through" entities in the case of many-to-many.
                     relationshipAttr.SetValue(entity, trackedRelationshipValue);
             }
             _dbSet.Add(entity);
@@ -163,18 +163,18 @@ private void DetachRelationships(TResource entity)
                 {
                     foreach (IIdentifiable single in collection.ToList())
                         _context.Entry(single).State = EntityState.Detached;
-                    /// detaching has many relationships is not sufficient to 
-                    /// trigger a full reload of relationships: the navigation 
-                    /// property actually needs to be nulled out, otherwise
-                    /// EF will still add duplicate instances to the collection
+                    // detaching has many relationships is not sufficient to 
+                    // trigger a full reload of relationships: the navigation 
+                    // property actually needs to be nulled out, otherwise
+                    // EF will still add duplicate instances to the collection
                     relationship.SetValue(entity, null);
                 }
                 else
                 {
                     _context.Entry(value).State = EntityState.Detached;
 
-                    /// temporary work around for https://github.com/aspnet/EntityFrameworkCore/issues/18621
-                    /// as soon as ef core 3.1 lands we can get rid of this again.
+                    // temporary work around for https://github.com/aspnet/EntityFrameworkCore/issues/18621
+                    // as soon as ef core 3.1 lands we can get rid of this again.
                     _context.Entry(entity).State = EntityState.Detached;
                 }
             }
@@ -192,16 +192,16 @@ public virtual async Task<TResource> UpdateAsync(TResource updatedEntity)
 
             foreach (var relationshipAttr in _targetedFields.Relationships)
             {
-                /// loads databasePerson.todoItems
+                // loads databasePerson.todoItems
                 LoadCurrentRelationships(databaseEntity, relationshipAttr);
-                /// trackedRelationshipValue is either equal to updatedPerson.todoItems,
-                /// or replaced with the same set (same ids) of todoItems from the EF Core change tracker,
-                /// which is the case if they were already tracked
+                // trackedRelationshipValue is either equal to updatedPerson.todoItems,
+                // or replaced with the same set (same ids) of todoItems from the EF Core change tracker,
+                // which is the case if they were already tracked
                 object trackedRelationshipValue = GetTrackedRelationshipValue(relationshipAttr, updatedEntity, out _);
-                /// loads into the db context any persons currently related
-                /// to the todoItems in trackedRelationshipValue
+                // loads into the db context any persons currently related
+                // to the todoItems in trackedRelationshipValue
                 LoadInverseRelationships(trackedRelationshipValue, relationshipAttr);
-                /// assigns the updated relationship to the database entity
+                // assigns the updated relationship to the database entity
                 //AssignRelationshipValue(databaseEntity, trackedRelationshipValue, relationshipAttr);
                 relationshipAttr.SetValue(databaseEntity, trackedRelationshipValue);
             }
@@ -395,17 +395,17 @@ private IIdentifiable AttachOrGetTracked(IIdentifiable relationshipValue)
 
             if (trackedEntity != null)
             {
-                /// there already was an instance of this type and ID tracked
-                /// by EF Core. Reattaching will produce a conflict, so from now on we 
-                /// will use the already attached instance instead. This entry might
-                /// contain updated fields as a result of business logic elsewhere in the application
+                // there already was an instance of this type and ID tracked
+                // by EF Core. Reattaching will produce a conflict, so from now on we 
+                // will use the already attached instance instead. This entry might
+                // contain updated fields as a result of business logic elsewhere in the application
                 return trackedEntity;
             }
 
-            /// the relationship pointer is new to EF Core, but we are sure
-            /// it exists in the database, so we attach it. In this case, as per
-            /// the json:api spec, we can also safely assume that no fields of 
-            /// this entity were updated.
+            // the relationship pointer is new to EF Core, but we are sure
+            // it exists in the database, so we attach it. In this case, as per
+            // the json:api spec, we can also safely assume that no fields of 
+            // this entity were updated.
             _context.Entry(relationshipValue).State = EntityState.Unchanged;
             return null;
         }

src/JsonApiDotNetCore/Extensions/DbContextExtensions.cs

@@ -75,7 +75,7 @@ public static async Task<IDbContextTransaction> GetCurrentOrCreateTransactionAsy
     /// If a transaction already exists, commit, rollback and dispose
     /// will not be called. It is assumed the creator of the original
     /// transaction should be responsible for disposal.
-    /// <summary>
+    /// </summary>
     internal struct SafeTransactionProxy : IDbContextTransaction
     {
         private readonly bool _shouldExecute;

src/JsonApiDotNetCore/Extensions/EntityFrameworkCoreExtension.cs

@@ -67,11 +67,6 @@ public static class IServiceCollectionExtensions
         /// <summary>
         /// Enabling JsonApiDotNetCore using the EF Core DbContext to build the ResourceGraph.
         /// </summary>
-        /// <typeparam name="TContext"></typeparam>
-        /// <param name="services"></param>
-        /// <param name="options"></param>
-        /// <param name="resources"></param>
-        /// <returns></returns>
         public static IServiceCollection AddJsonApi<TDbContext>(this IServiceCollection services,
                                                     Action<JsonApiOptions> options = null,
                                                     Action<IServiceDiscoveryFacade> discovery = null,

src/JsonApiDotNetCore/Extensions/IServiceCollectionExtensions.cs

@@ -21,10 +21,6 @@ public static class IServiceCollectionExtensions
         /// <summary>
         /// Enabling JsonApiDotNetCore using manual declaration to build the ResourceGraph.
         /// </summary>
-        /// <param name="services"></param>
-        /// <param name="options"></param>
-        /// <param name="resources"></param>
-        /// <returns></returns>
         public static IServiceCollection AddJsonApi(this IServiceCollection services,
                                                     Action<JsonApiOptions> options = null,
                                                     Action<IServiceDiscoveryFacade> discovery = null,

src/JsonApiDotNetCore/Formatters/JsonApiReader.cs

@@ -69,7 +69,7 @@ public async  Task<InputFormatterResult> ReadAsync(InputFormatterContext context
             }
         }
 
-        /// <summary> Checks if the deserialized payload has an ID included </summary
+        /// <summary> Checks if the deserialized payload has an ID included </summary>
         private bool CheckForId(object model)
         {
             if (model == null) return false;
@@ -84,7 +84,7 @@ private bool CheckForId(object model)
             return false;
         }
 
-        /// <summary> Checks if the elements in the deserialized payload have an ID included </summary
+        /// <summary> Checks if the elements in the deserialized payload have an ID included </summary>
         private bool CheckForId(IList modelList)
         {
             foreach (var model in modelList)

src/JsonApiDotNetCore/Graph/TypeLocator.cs

@@ -113,9 +113,9 @@ public static (Type implementation, Type registrationInterface) GetGenericInterf
         /// <param name="openGenericType">The open generic type, e.g. `typeof(ResourceDefinition&lt;&gt;)`</param>
         /// <param name="genericArguments">Parameters to the generic type</param>
         /// <example>
-        /// <code>
+        /// <code><![CDATA[
         /// GetDerivedGenericTypes(assembly, typeof(ResourceDefinition<>), typeof(Article))
-        /// </code>
+        /// ]]></code>
         /// </example>
         public static IEnumerable<Type> GetDerivedGenericTypes(Assembly assembly, Type openGenericType, params Type[] genericArguments)
         {
@@ -127,7 +127,7 @@ public static IEnumerable<Type> GetDerivedGenericTypes(Assembly assembly, Type o
         /// Get all derivatives of the specified type.
         /// </summary>
         /// <param name="assembly">The assembly to search</param>
-        /// <param name="openGenericType">The inherited type</param>
+        /// <param name="inheritedType">The inherited type</param>
         /// <example>
         /// <code>
         /// GetDerivedGenericTypes(assembly, typeof(DbContext))

src/JsonApiDotNetCore/Hooks/Execution/HookExecutorHelper.cs

@@ -37,19 +37,19 @@ public HookExecutorHelper(IGenericServiceFactory genericProcessorFactory,
         /// <inheritdoc/>
         public IResourceHookContainer GetResourceHookContainer(RightType rightType, ResourceHook hook = ResourceHook.None)
         {
-            /// checking the cache if we have a reference for the requested container, 
-            /// regardless of the hook we will use it for. If the value is null, 
-            /// it means there was no implementation IResourceHookContainer at all, 
-            /// so we need not even bother.
+            // checking the cache if we have a reference for the requested container, 
+            // regardless of the hook we will use it for. If the value is null, 
+            // it means there was no implementation IResourceHookContainer at all, 
+            // so we need not even bother.
             if (!_hookContainers.TryGetValue(rightType, out IResourceHookContainer container))
             {
                 container = (_genericProcessorFactory.Get<IResourceHookContainer>(typeof(ResourceDefinition<>), rightType));
                 _hookContainers[rightType] = container;
             }
             if (container == null) return container;
 
-            /// if there was a container, first check if it implements the hook we 
-            /// want to use it for.
+            // if there was a container, first check if it implements the hook we 
+            // want to use it for.
             List<ResourceHook> targetHooks;
             if (hook == ResourceHook.None)
             {

src/JsonApiDotNetCore/Hooks/IResourceHookContainer.cs

@@ -1,4 +1,4 @@
-using System.Collections.Generic;
+using System.Collections.Generic;
 using JsonApiDotNetCore.Models;
 using JsonApiDotNetCore.Services;
 
@@ -143,7 +143,7 @@ public interface IUpdateHookContainer<TResource> where TResource : class, IIdent
         /// <para />
         /// If relationships were updated with the updated entities, this will
         /// be reflected by the corresponding NavigationProperty being set. 
-        /// For each of these relationships, the <see cref="ResourceDefinition{T}.AfterUpdateRelationship(IRelationshipsDictionary{T}, ResourcePipeline"/>
+        /// For each of these relationships, the <see cref="ResourceDefinition{T}.AfterUpdateRelationship(IRelationshipsDictionary{T}, ResourcePipeline)"/>
         /// hook is fired after the execution of this hook.
         /// </summary>
         /// <param name="entities">The unique set of affected entities.</param>
@@ -211,7 +211,7 @@ public interface IDeleteHookContainer<TResource> where TResource : class, IIdent
         /// </summary>
         /// <param name="entities">The unique set of affected entities.</param>
         /// <param name="pipeline">An enum indicating from where the hook was triggered.</param>
-        /// <param name="succeeded">If set to <c>true</c> if the deletion was succeeded in the repository layer.</param>
+        /// <param name="succeeded">If set to <c>true</c> the deletion succeeded in the repository layer.</param>
         void AfterDelete(HashSet<TResource> entities, ResourcePipeline pipeline, bool succeeded);
     }
 

src/JsonApiDotNetCore/Hooks/IResourceHookExecutor.cs

@@ -1,4 +1,4 @@
-using System.Collections.Generic;
+using System.Collections.Generic;
 using JsonApiDotNetCore.Models;
 using JsonApiDotNetCore.Services;
 
@@ -67,6 +67,7 @@ public interface IDeleteHookExecutor
         /// <param name="pipeline">An enum indicating from where the hook was triggered.</param>
         /// <typeparam name="TResource">The type of the root entities</typeparam>
         IEnumerable<TResource> BeforeDelete<TResource>(IEnumerable<TResource> entities, ResourcePipeline pipeline) where TResource : class, IIdentifiable;
+
         /// <summary>
         /// Executes the After Cycle by firing the appropriate hooks if they are implemented. 
         /// <para />
@@ -75,6 +76,7 @@ public interface IDeleteHookExecutor
         /// </summary>
         /// <param name="entities">Target entities for the Before cycle.</param>
         /// <param name="pipeline">An enum indicating from where the hook was triggered.</param>
+        /// <param name="succeeded">If set to <c>true</c> the deletion succeeded.</param>
         /// <typeparam name="TResource">The type of the root entities</typeparam>
         void AfterDelete<TResource>(IEnumerable<TResource> entities, ResourcePipeline pipeline, bool succeeded) where TResource : class, IIdentifiable;
     }
@@ -92,8 +94,8 @@ public interface IReadHookExecutor
         /// entities as well as any related relationship.
         /// </summary>
         /// <param name="pipeline">An enum indicating from where the hook was triggered.</param>
-        /// <param name="stringId">StringId of the requested entity in the case of 
-        /// <see cref="DefaultResourceService{X, Y, Z}.GetAsync(Z)"/>.</param>
+        /// <param name="stringId">StringId of the requested entity in the case of
+        /// <see cref="DefaultResourceService{TResource,TId}.GetAsync(TId)"/>.</param>
         /// <typeparam name="TResource">The type of the request entity</typeparam>
         void BeforeRead<TResource>(ResourcePipeline pipeline, string stringId = null) where TResource : class, IIdentifiable;
         /// <summary>
@@ -166,4 +168,4 @@ public interface IOnReturnHookExecutor
         /// <typeparam name="TResource">The type of the root entities</typeparam>
         IEnumerable<TResource> OnReturn<TResource>(IEnumerable<TResource> entities, ResourcePipeline pipeline) where TResource : class, IIdentifiable;
     }
-}
+}