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

Logging Source Generator fails to compile due to CS8795 error with file-scoped namespaces #57880

Closed
martincostello opened this issue Aug 21, 2021 · 7 comments

Comments

@martincostello
Copy link
Member

martincostello commented Aug 21, 2021

Description

If a class used to provide the partial methods for the Logging Source Generator uses file-scoped namespaces, the project fails to compile with errors similar to the below.

Project\Logger.cs(11,36): error CS8795: Partial method 'Logger.Greeting(ILogger, string)' must have an implementation part because it has accessibility modifiers. [Project\Project.csproj]
Project\Microsoft.Extensions.Logging.Generators\Microsoft.Extensions.Logging.Generators.LoggerMessageGenerator\LoggerMessage.g.cs(11,36): error CS0759: No defining declaration found for implementing declaration of partial method 'Logger.Greeting(ILogger, string)' [Project\Project.csproj]

Minimal repro

Project.csproj

<Project Sdk="Microsoft.NET.Sdk">
  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFramework>net6.0</TargetFramework>
    <ImplicitUsings>enable</ImplicitUsings>
    <Nullable>enable</Nullable>
  </PropertyGroup>
  <ItemGroup>
    <PackageReference Include="Microsoft.Extensions.DependencyInjection" Version="6.0.0-rc.1.21420.7" />
    <PackageReference Include="Microsoft.Extensions.Logging" Version="6.0.0-rc.1.21420.7" />
    <PackageReference Include="Microsoft.Extensions.Logging.Console" Version="6.0.0-rc.1.21420.7" />
  </ItemGroup>
</Project>

Program.cs

using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Logging;
using MyLibrary;

using var serviceProvider = new ServiceCollection()
    .AddLogging(builder => builder.AddConsole())
    .BuildServiceProvider();

var loggerFactory = serviceProvider.GetRequiredService<ILoggerFactory>();
var logger = loggerFactory.CreateLogger("MyLogger");

Logger.Greeting(logger, "World");

Logger.cs

using Microsoft.Extensions.Logging;

namespace MyLibrary;

internal partial class Logger
{
    [LoggerMessage(EventId = 1, Level = LogLevel.Information, Message = "Hello {Name}!")]
    public static partial void Greeting(ILogger logger, string name);
}

Expected Output

Project> dotnet run
info: MyLogger[1]
      Hello World!

Configuration

.NET SDK 6.0.100-rc.1.21420.39
Microsoft.Extensions.Logging 6.0.0-rc.1.21420.7

Regression?

No.

@dotnet-issue-labeler dotnet-issue-labeler bot added area-Extensions-Logging untriaged New issue has not been triaged by the area owner labels Aug 21, 2021
@ghost
Copy link

ghost commented Aug 21, 2021

Tagging subscribers to this area: @maryamariyan
See info in area-owners.md if you want to be subscribed.

Issue Details

Description

If a class used to provide the partial methods for the Logging Source Generator uses file-scoped namespaces, the project fails to compile with errors similar to the below.

Project\Logger.cs(11,36): error CS8795: Partial method 'Logger.Greeting(ILogger, string)' must have an implementation part because it has accessibility modifiers. [Project\Project.csproj]
Project\Microsoft.Extensions.Logging.Generators\Microsoft.Extensions.Logging.Generators.LoggerMessageGenerator\LoggerMessage.g.cs(11,36): error CS0759: No defining declaration found for implementing declaration of partial method 'Logger.Greeting(ILogger, string)' [Project\Project.csproj]

Minimal repro

Project.csproj

<Project Sdk="Microsoft.NET.Sdk">
  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFramework>net6.0</TargetFramework>
    <ImplicitUsings>enable</ImplicitUsings>
    <Nullable>enable</Nullable>
  </PropertyGroup>
  <ItemGroup>
    <PackageReference Include="Microsoft.Extensions.DependencyInjection" Version="6.0.0-rc.1.21420.7" />
    <PackageReference Include="Microsoft.Extensions.Logging" Version="6.0.0-rc.1.21420.7" />
    <PackageReference Include="Microsoft.Extensions.Logging.Console" Version="6.0.0-rc.1.21420.7" />
  </ItemGroup>
</Project>

Program.cs

using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Logging;
using MyLibrary;

using var serviceProvider = new ServiceCollection()
    .AddLogging(builder => builder.AddConsole())
    .BuildServiceProvider();

var loggerFactory = serviceProvider.GetRequiredService<ILoggerFactory>();
var logger = loggerFactory.CreateLogger("MyLogger");

Logger.Greeting(logger, "World");

Logger.cs

using Microsoft.Extensions.Logging;

namespace MyLibrary;

internal partial class Logger
{
    [LoggerMessage(EventId = 1, Level = LogLevel.Information, Message = "Hello {Name}!")]
    public static partial void Greeting(ILogger logger, string name);
}

Expected Output

Project> dotnet run
info: MyLogger[1]
      Hello World!

Configuration

.NET SDK 6.0.100-rc.1.21420.39
Microsoft.Extensions.Logging 6.0.0-rc.1.21420.7

Regression?

No.

Author: martincostello
Assignees: -
Labels:

untriaged, area-Extensions-Logging

Milestone: -

@maryamariyan maryamariyan removed the untriaged New issue has not been triaged by the area owner label Aug 21, 2021
@ghost
Copy link

ghost commented Aug 22, 2021

Potential fix in LoggerMessageGenerator.Parser.cs line 410 :

// determine the namespace the class is declared in, if any
SyntaxNode? potentialNamespaceParent = classDec.Parent;
- while (potentialNamespaceParent != null && potentialNamespaceParent is not NamespaceDeclarationSyntax)
+ while (potentialNamespaceParent != null &&
+    potentialNamespaceParent is not NamespaceDeclarationSyntax &&
+    potentialNamespaceParent is not FileScopedNamespaceDeclarationSyntax)
{
    potentialNamespaceParent = potentialNamespaceParent.Parent;
}
- if (potentialNamespaceParent is NamespaceDeclarationSyntax namespaceParent)
+ if (potentialNamespaceParent is BaseNamespaceDeclarationSyntax namespaceParent)
{

The type FileScopedNamespaceDeclarationSyntax seems to be available from Microsoft.CodeAnalysis.CSharp version 4.0.0-3.final

@davidfowl
Copy link
Member

Seems like we should fix this for 6.0.0

cc @maryamariyan @ericstj @eerhardt

martincostello added a commit to martincostello/runtime that referenced this issue Aug 22, 2021
Handle namespace not being emitted when file-scoped
namespaces are used for a log class.
Relates to dotnet#57880.
@martincostello
Copy link
Member Author

I've opened a PR with the proposed fix #57880 (comment) in #57894.

@maryamariyan
Copy link
Member

maryamariyan commented Sep 10, 2021

We merged the fix to main, will need to backport to 6.0.0 and test/fix for JSON if needed too.

/cc @layomia

@martincostello
Copy link
Member Author

JSON seems fine: #57894 (comment)

maryamariyan pushed a commit to maryamariyan/runtime that referenced this issue Sep 14, 2021
Handle namespace not being emitted when file-scoped
namespaces are used for a log class.
Relates to dotnet#57880.
ericstj pushed a commit that referenced this issue Sep 17, 2021
…spaces (#57894) (#59100)

* Handle file-scoped namespaces (#57894)

Handle namespace not being emitted when file-scoped
namespaces are used for a log class.
Relates to #57880.

* Fix build issue

Co-authored-by: Martin Costello <[email protected]>
@tarekgh
Copy link
Member

tarekgh commented Sep 23, 2021

Has been fixed by #57894

@tarekgh tarekgh closed this as completed Sep 23, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Nov 3, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants