Enable CS1591 for projects that don't use intellisense package XML file #84917

ViktorHofer · 2023-04-17T09:11:58Z

Noticed that while looking at the intellisense.targets file that projects like Microsoft.Extensions which don't use the Microsoft.Private.Intellisense intellisense package XML file still set NoWarn+=CS1591.

WIP: This will probably result in some projects raising errors that we need to fix.

carlossanlop · 2023-04-18T22:18:23Z

From our chat, you mentioned we can also ignore Microsoft.Extensions.Specifications.Tests because it isn't critical to offer API as it's just a test dependency tool.

BTW, I couldn't find System.Numerics.Tensors in dotnet-api-docs. Turns out it is non-shipping (Tanner confirmed), and we remove it on purpose from the ref assemblies drop we send to them.

I mention it because I was trying to backport the docs, but couldn't find anything. So I guess we need to ignore the CS warning in that assembly.

carlossanlop · 2023-04-19T03:38:53Z

The RateLimiter APIs live in the ASP.NET API docs repo, and they are undocumented there. If we want to fill the missing documentation items, it'll have to be done manually in triple slash directly.

https://github.com/dotnet/AspNetApiDocs/blob/main/aspnet-core/xml/System.Threading.RateLimiting

carlossanlop

Couple of unnecessary full namespaces.

src/libraries/Microsoft.Extensions.Hosting.WindowsServices/src/WindowsServiceLifetime.cs

src/libraries/Microsoft.Extensions.Hosting.Systemd/src/ISystemdNotifier.cs

ghost · 2023-05-24T23:03:08Z

Draft Pull Request was automatically closed for 30 days of inactivity. Please let us know if you'd like to reopen it.

ViktorHofer · 2023-05-25T10:01:36Z

Reopening as we want to continue this work when we have free cycles again (soon).

…sproj

src/libraries/System.Threading.RateLimiting/src/System.Threading.RateLimiting.csproj

carlossanlop

Thanks. Left some comments.

carlossanlop · 2023-06-16T20:15:26Z

src/libraries/Microsoft.Extensions.Hosting.Systemd/src/SystemdLifetime.cs

+        /// </param>
+        /// <returns>
+        /// A task that represents the asynchronous stop operation.
+        /// </returns>
        public Task StopAsync(CancellationToken cancellationToken)
        {
            return Task.CompletedTask;


The added comments do not match what the method does. This is only returning the object that represents a completed task.

You added those in one of the commits. I assume they are just a straight port from docs?

Not sure I understand. What commits are you referring to?

You contributed to this PR. See the commits tab.

3013103

Oh. I just noticed the date of this PR.

Yes, I ported them using the tool. They exist this way in MS Docs: https://learn.microsoft.com/en-us/dotnet/api/microsoft.extensions.hosting.systemd.systemdlifetime.stopasync?view=dotnet-plat-ext-3.1

@gewarren I suppose the documentation of this API needs to say something along the lines of "This API does nothing except returning a completed task". Is this correct?

@ViktorHofer we can address it in a separate PR.

@carlossanlop Yes the summary could be something like "Returns a task that has already completed."

carlossanlop · 2023-06-16T20:19:03Z

src/libraries/Microsoft.Extensions.Hosting.WindowsServices/src/WindowsServiceLifetimeOptions.cs

@@ -5,6 +5,9 @@

 namespace Microsoft.Extensions.Hosting
 {
+    /// <summary>
+    /// Options to configure the lifetime of a windows service.


Suggested change

/// Options to configure the lifetime of a windows service.

/// Options to configure the lifetime of a Windows service.

carlossanlop · 2023-06-16T20:20:59Z

....Specification.Tests/src/Microsoft.Extensions.DependencyInjection.Specification.Tests.csproj

@@ -9,6 +9,8 @@
    <IsTrimmable>false</IsTrimmable>
    <EnableAOTAnalyzer>false</EnableAOTAnalyzer>
    <NoWarn>$(NoWarn);CA1852</NoWarn>
+    <!-- Public API not fully documented. -->
+    <NoWarn>$(NoWarn);1591</NoWarn>


FYI @dotnet/area-extensions-dependencyinjection area owners: This assembly is missing public documentation.

carlossanlop · 2023-06-16T20:21:48Z

src/libraries/System.Numerics.Tensors/src/System.Numerics.Tensors.csproj

@@ -2,6 +2,8 @@
  <PropertyGroup>
    <TargetFrameworks>$(NetCoreAppCurrent);$(NetCoreAppPrevious);$(NetCoreAppMinimum);netstandard2.0;$(NetFrameworkMinimum)</TargetFrameworks>
    <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
+    <!-- Public API not fully documented. -->
+    <NoWarn>$(NoWarn);1591</NoWarn>


FYI @dotnet/area-system-numerics-tensors area owners, this assembly is missing public documentation.

carlossanlop · 2023-06-16T20:22:17Z

src/libraries/System.Speech/src/System.Speech.csproj

@@ -7,6 +7,8 @@
    <NoWarn>$(NoWarn);CS0649;SA1129;IDE0059;IDE0060;CA1822;CA1852</NoWarn>
    <Nullable>annotations</Nullable>
    <IsTrimmable>false</IsTrimmable>
+    <!-- Public API not fully documented. -->
+    <NoWarn>$(NoWarn);1591</NoWarn>


FYI @dotnet/area-system-speech area owners: This assembly is missing public documentation.

carlossanlop · 2023-06-16T20:24:22Z

eng/intellisense.targets

+         - the assembly is a PNSE assembly. -->
+    <NoWarn Condition="'$(IntellisensePackageXmlFilePath)' != '' or
+                       '$(IsPrivateAssembly)' == 'true' or
+                       '$(GeneratePlatformNotSupportedAssemblyMessage)' != ''">$(NoWarn);1591</NoWarn>


What about IsPartialFacadeAssembly?

The compiler doesn't warn for undocumented types referenced via type forwards.

ViktorHofer · 2023-06-16T21:24:56Z

@carlossanlop would you mind if we react to additional feedback in a follow-up change and meanwhile get this in? I would like to have this validation in place in main. I could imagine that with a new assembly or new API added to one of the source-of-truth assemblies, this PR would start failing again.

src/libraries/Microsoft.Bcl.AsyncInterfaces/src/Microsoft.Bcl.AsyncInterfaces.csproj

Enable CS1591 for projects that don't use intellisense package XML file

f4822de

ViktorHofer requested a review from carlossanlop April 17, 2023 09:11

ViktorHofer self-assigned this Apr 17, 2023

dotnet-issue-labeler bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Apr 17, 2023

build-analysis bot mentioned this pull request Apr 17, 2023

Tracking issue for CI build timeouts #76454

Closed

ViktorHofer added 4 commits April 17, 2023 16:45

Update Directory.Build.targets

2abc5e3

Update Directory.Build.targets

0fc4d5d

Update intellisense.targets

b4f9296

Update intellisense.targets

9a8b087

ViktorHofer assigned carlossanlop Apr 17, 2023

Update Microsoft.NETCore.Platforms.csproj

e8a4f46

ViktorHofer added bug area-Infrastructure-libraries and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Apr 18, 2023

ViktorHofer added this to the 8.0.0 milestone Apr 18, 2023

ViktorHofer added the blocking-release label Apr 18, 2023

carlossanlop added 2 commits April 18, 2023 21:41

Backport docs: Microsoft.Extensions.Hosting.WindowsServices

e14424e

Backport docs: Microsoft.Extensions.Hosting.Systemd

3013103

carlossanlop reviewed Apr 19, 2023

View reviewed changes

src/libraries/Microsoft.Extensions.Hosting.WindowsServices/src/WindowsServiceLifetime.cs Outdated Show resolved Hide resolved

src/libraries/Microsoft.Extensions.Hosting.WindowsServices/src/WindowsServiceLifetime.cs Outdated Show resolved Hide resolved

Remove unnecessary full namespaces in triple slash.

11b1964

ViktorHofer commented Apr 19, 2023

View reviewed changes

src/libraries/Microsoft.Extensions.Hosting.Systemd/src/ISystemdNotifier.cs Outdated Show resolved Hide resolved

Fix xml comment formatting.

dc3c42d

ghost closed this May 24, 2023

ViktorHofer reopened this May 25, 2023

build-analysis bot mentioned this pull request May 25, 2023

Build on Windows Fails sometimes with fatal error C1090: PDB API call failed #48070

Open

Merge branch 'main' into ViktorHofer-patch-2

d65a202

ViktorHofer added 5 commits June 15, 2023 11:28

Update System.Numerics.Tensors.csproj

ae47053

Update Microsoft.Extensions.DependencyInjection.Specification.Tests.c…

38ab6e7

…sproj

Update System.Threading.RateLimiting.csproj

0f231ae

Update WindowsServiceLifetimeOptions.cs

c4e6b1d

Update SystemdNotifier.cs

f880b2d

ViktorHofer mentioned this pull request Jun 15, 2023

System.Threading.RateLimiting is missing XML documentation for public API #87604

Closed

ViktorHofer commented Jun 15, 2023

View reviewed changes

src/libraries/System.Threading.RateLimiting/src/System.Threading.RateLimiting.csproj Outdated Show resolved Hide resolved

ViktorHofer marked this pull request as ready for review June 15, 2023 10:08

Update Microsoft.Bcl.AsyncInterfaces.csproj

07c608e

ViktorHofer requested a review from carlossanlop June 15, 2023 10:38

Update System.Speech.csproj

32cf743

This was referenced Jun 15, 2023

Could not load file or assembly 'Microsoft.CodeAnalysis.NetAnalyzers #84995

Closed

Unable to load Analyzer assembly .../Microsoft.CodeAnalysis.Analyzers.dll : Not a valid assembly #85082

Closed

Update System.Threading.RateLimiting.csproj

182846e

carlossanlop reviewed Jun 16, 2023

View reviewed changes

This was referenced Jun 16, 2023

Microsoft.Extensions.DependencyInjection.Specification.Tests is missing API documentation #87709

Closed

System.Numerics.Tensors is missing API documentation #87710

Closed

System.Speech is missing API documentation #87711

Closed

carlossanlop approved these changes Jun 16, 2023

View reviewed changes

ViktorHofer merged commit 7d8db25 into main Jun 17, 2023

ViktorHofer deleted the ViktorHofer-patch-2 branch June 17, 2023 07:35

ViktorHofer mentioned this pull request Jun 17, 2023

Microsoft.Bcl.AsyncInterfaces is missing API documentation #87722

Closed

ViktorHofer commented Jun 17, 2023

View reviewed changes

src/libraries/Microsoft.Bcl.AsyncInterfaces/src/Microsoft.Bcl.AsyncInterfaces.csproj Show resolved Hide resolved

ghost locked as resolved and limited conversation to collaborators Jul 17, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable CS1591 for projects that don't use intellisense package XML file #84917

Enable CS1591 for projects that don't use intellisense package XML file #84917

ViktorHofer commented Apr 17, 2023

carlossanlop commented Apr 18, 2023 •

edited

Loading

carlossanlop commented Apr 19, 2023 •

edited

Loading

carlossanlop left a comment

ghost commented May 24, 2023

ViktorHofer commented May 25, 2023

carlossanlop left a comment

carlossanlop Jun 16, 2023

ViktorHofer Jun 16, 2023 •

edited

Loading

carlossanlop Jun 16, 2023

ViktorHofer Jun 16, 2023 •

edited

Loading

carlossanlop Jun 16, 2023

carlossanlop Jun 16, 2023

gewarren Jun 16, 2023

carlossanlop Jun 16, 2023

carlossanlop Jun 16, 2023

ViktorHofer Jun 16, 2023

carlossanlop Jun 16, 2023

ViktorHofer Jun 16, 2023

carlossanlop Jun 16, 2023

ViktorHofer Jun 16, 2023

carlossanlop Jun 16, 2023

ViktorHofer Jun 16, 2023

ViktorHofer commented Jun 16, 2023

	/// Options to configure the lifetime of a windows service.
	/// Options to configure the lifetime of a Windows service.

Enable CS1591 for projects that don't use intellisense package XML file #84917

Enable CS1591 for projects that don't use intellisense package XML file #84917

Conversation

ViktorHofer commented Apr 17, 2023

carlossanlop commented Apr 18, 2023 • edited Loading

carlossanlop commented Apr 19, 2023 • edited Loading

carlossanlop left a comment

Choose a reason for hiding this comment

ghost commented May 24, 2023

ViktorHofer commented May 25, 2023

carlossanlop left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ViktorHofer Jun 16, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ViktorHofer Jun 16, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ViktorHofer commented Jun 16, 2023

carlossanlop commented Apr 18, 2023 •

edited

Loading

carlossanlop commented Apr 19, 2023 •

edited

Loading

ViktorHofer Jun 16, 2023 •

edited

Loading

ViktorHofer Jun 16, 2023 •

edited

Loading