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

Enable internal classes #43

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
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
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
<PackageLicenseExpression>MIT</PackageLicenseExpression>
<EnableNETAnalyzers>True</EnableNETAnalyzers>
<AnalysisLevel>latest-Recommended</AnalysisLevel>
<Version>2.3.0</Version>
<Version>3.0.0</Version>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, definitely needed.
I would suggest also adding examples, changelog and your name to Readme.md - especially calling out that you fixed a bug where classes without access modifier where wrongly set to public instead of internal.

Copy link
Author

Choose a reason for hiding this comment

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

Good point!

<PackageReadmeFile>README.md</PackageReadmeFile>
<TreatWarningsAsErrors>true</TreatWarningsAsErrors>
<NoWarn>1701;1702;NU5128</NoWarn>
Expand Down
5 changes: 4 additions & 1 deletion AutomaticInterface/AutomaticInterface/Builder.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using System;
using System.Collections.Generic;
using System.Globalization;
using System.Linq;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp;
Expand All @@ -25,7 +26,9 @@ is not ClassDeclarationSyntax classSyntax

var interfaceName = $"I{classSyntax.GetClassName()}";

var interfaceGenerator = new InterfaceBuilder(namespaceName, interfaceName);
var accessSpecifier = typeSymbol.DeclaredAccessibility.ToString().ToLower(new CultureInfo("C"));

var interfaceGenerator = new InterfaceBuilder(namespaceName, interfaceName, accessSpecifier);

interfaceGenerator.AddUsings(GetUsings(typeSymbol));
interfaceGenerator.AddClassDocumentation(GetDocumentationForClass(classSyntax));
Expand Down
4 changes: 2 additions & 2 deletions AutomaticInterface/AutomaticInterface/InterfaceBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ internal sealed record MethodInfo(

public record EventInfo(string Name, string Type, string Documentation);

public class InterfaceBuilder(string nameSpaceName, string interfaceName)
public class InterfaceBuilder(string nameSpaceName, string interfaceName, string accessSpecifier="public")
{
private const string Autogenerated = """
//--------------------------------------------------------------------------------------------------
Expand Down Expand Up @@ -117,7 +117,7 @@ public string Build()
cb.AppendAndNormalizeMultipleLines(classDocumentation);

cb.AppendLine($"[GeneratedCode(\"AutomaticInterface\", \"\")]");
cb.AppendLine($"public partial interface {interfaceName}{genericType}");
cb.AppendLine($"{accessSpecifier} partial interface {interfaceName}{genericType}");
cb.AppendLine("{");

cb.Indent();
Expand Down
230 changes: 230 additions & 0 deletions AutomaticInterface/Tests/AccessibilityTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,230 @@
using System;
using System.Linq;
using AutomaticInterface;
using FluentAssertions;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp;
using Xunit;

namespace Tests;

public partial class AccessibilityTests
{
private static string GenerateCode(string code)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hm, this dublicates code in the original tests.
Maybe use the same partial class tests I've used like in AutomaticInterface/Tests/GeneratorsTests.MethodParameters.cs?

{
var syntaxTree = CSharpSyntaxTree.ParseText(code);
var references = AppDomain
.CurrentDomain.GetAssemblies()
.Where(assembly => !assembly.IsDynamic)
.Select(assembly => MetadataReference.CreateFromFile(assembly.Location))
.Cast<MetadataReference>();

var compilation = CSharpCompilation.Create(
"SourceGeneratorTests",
new[] { syntaxTree },
references,
new(OutputKind.DynamicallyLinkedLibrary)
);

var generator = new AutomaticInterfaceGenerator();

CSharpGeneratorDriver
.Create(generator)
.RunGeneratorsAndUpdateCompilation(
compilation,
out var outputCompilation,
out var diagnostics
);

diagnostics.Where(d => d.Severity == DiagnosticSeverity.Error).Should().BeEmpty();

return outputCompilation.SyntaxTrees.Skip(1).LastOrDefault()?.ToString();
}

[Fact]
public void AddsPublicMethodToInterface()
Copy link
Collaborator

Choose a reason for hiding this comment

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

What does this test test?
Seems to me already tested by existing tests?

{
const string code = """

using AutomaticInterfaceAttribute;
using System.IO;

namespace AutomaticInterfaceExample
{

[GenerateAutomaticInterface]
public class DemoClass
{

public string Hello(){return "";}
}
}

""";

const string expected = """
//--------------------------------------------------------------------------------------------------
// <auto-generated>
// This code was generated by a tool.
//
// Changes to this file may cause incorrect behavior and will be lost if the code is regenerated.
// </auto-generated>
//--------------------------------------------------------------------------------------------------

using System.CodeDom.Compiler;
using AutomaticInterfaceAttribute;
using System.IO;

namespace AutomaticInterfaceExample
{
[GeneratedCode("AutomaticInterface", "")]
public partial interface IDemoClass
{
/// <inheritdoc />
string Hello();

}
}

""";
GenerateCode(code).Should().Be(expected);
}

[Fact]
public void IgnoresNotPublicMethodToInterface()
{
const string code = """

using AutomaticInterfaceAttribute;
using System.IO;

namespace AutomaticInterfaceExample
{

[GenerateAutomaticInterface]
public class DemoClass
{
private string Hello(string x, int y, double z){return x;}
internal string Hello2(string x, int y, double z){return x;}
}
}

""";

const string expected = """
//--------------------------------------------------------------------------------------------------
// <auto-generated>
// This code was generated by a tool.
//
// Changes to this file may cause incorrect behavior and will be lost if the code is regenerated.
// </auto-generated>
//--------------------------------------------------------------------------------------------------

using System.CodeDom.Compiler;
using AutomaticInterfaceAttribute;
using System.IO;

namespace AutomaticInterfaceExample
{
[GeneratedCode("AutomaticInterface", "")]
public partial interface IDemoClass
{
}
}

""";
GenerateCode(code).Should().Be(expected);
}

[Fact]
public void WorksWithInternal()
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't see internal here?

{
const string code = """

using AutomaticInterfaceAttribute;
using System.Threading.Tasks;

namespace AutomaticInterfaceExample;

[GenerateAutomaticInterface]
internal class InternalClass
{
public int AProperty { get; set; }
}

""";

const string expected = """
//--------------------------------------------------------------------------------------------------
// <auto-generated>
// This code was generated by a tool.
//
// Changes to this file may cause incorrect behavior and will be lost if the code is regenerated.
// </auto-generated>
//--------------------------------------------------------------------------------------------------

using System.CodeDom.Compiler;
using AutomaticInterfaceAttribute;
using System.Threading.Tasks;

namespace AutomaticInterfaceExample
{
[GeneratedCode("AutomaticInterface", "")]
internal partial interface IInternalClass
{
/// <inheritdoc />
int AProperty { get; set; }

}
}

""";
GenerateCode(code).Should().Be(expected);
}

[Fact]
public void WorksWithDefaultAccessModifier()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Looks like the above test?

{
const string code = """

using AutomaticInterfaceAttribute;
using System.Threading.Tasks;

namespace AutomaticInterfaceExample;

[GenerateAutomaticInterface]
class InternalClass
{
public int AProperty { get; set; }
}

""";

const string expected = """
//--------------------------------------------------------------------------------------------------
// <auto-generated>
// This code was generated by a tool.
//
// Changes to this file may cause incorrect behavior and will be lost if the code is regenerated.
// </auto-generated>
//--------------------------------------------------------------------------------------------------

using System.CodeDom.Compiler;
using AutomaticInterfaceAttribute;
using System.Threading.Tasks;

namespace AutomaticInterfaceExample
{
[GeneratedCode("AutomaticInterface", "")]
internal partial interface IInternalClass
{
/// <inheritdoc />
int AProperty { get; set; }

}
}

""";
GenerateCode(code).Should().Be(expected);
}
}
Loading