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

[WIP] C# 8 nullable reference type support #99

Closed
wants to merge 9 commits into from
Closed

[WIP] C# 8 nullable reference type support #99

wants to merge 9 commits into from

Conversation

sungam3r
Copy link
Member

fixes #54
tests for NRT

@@ -9,8 +9,11 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "PublicApiGenerator", "Publi
EndProject
Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Solution Items", "Solution Items", "{52361C5B-F247-42C5-8455-BD1DEDB5C903}"
ProjectSection(SolutionItems) = preProject
..\appveyor.yml = ..\appveyor.yml
..\build.ps1 = ..\build.ps1
..\.gitattributes = ..\.gitattributes
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for cleaning this up

@@ -250,7 +250,7 @@ public static void Main(string[] args)
var asm = Assembly.LoadFile(fullPath);
File.WriteAllText(outputPath, ApiGenerator.GeneratePublicApi(asm));
var destinationFilePath = Path.Combine(outputDirectory, Path.GetFileName(outputPath));
if(File.Exists(destinationFilePath))
if (File.Exists(destinationFilePath))
Copy link
Member

Choose a reason for hiding this comment

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

Not really related but I'm ok with the change. I'll probably align code styles anyway after merging

src/PublicApiGenerator/CodeTypeReferenceBuilder.cs Outdated Show resolved Hide resolved
var typeName = GetTypeNameCore(type, nullabilityMap, nullable);

if (nullable)
typeName = CSharpAlias.Get(typeName) + "?";
Copy link
Member

Choose a reason for hiding this comment

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

Kudos to keep it consistent with the other string concatenations and not use interpolation yet.

src/PublicApiGenerator/CodeTypeReferenceBuilder.cs Outdated Show resolved Hide resolved
src/PublicApiGenerator/NullableContext.cs Show resolved Hide resolved
@sungam3r sungam3r changed the title C# 8 nullable reference type support [WIP] C# 8 nullable reference type support Oct 26, 2019
@sungam3r
Copy link
Member Author

Need more tests for generic constraints and base classes.

}");
}

[Fact(Skip = "There is a problem with these tests - the attribute says 1 instead of 2")]
Copy link
Member Author

Choose a reason for hiding this comment

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

Help is appreciated.

Copy link
Member

Choose a reason for hiding this comment

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

I pinged Immo over twitter. Maybe he has some input

Copy link
Member Author

Choose a reason for hiding this comment

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

In fact, we found a place where these attributes are stored. And it seems that they are hard to get.

@danielmarbach
Copy link
Member

CI build on Windows should be fixed. Maybe rebase on top of master?

@sungam3r
Copy link
Member Author

Can this be done from the web interface?

@danielmarbach
Copy link
Member

Sorry that I was assuming you know how to rebase. Usually I rebase in Gitextensions or the command line. You can only do it on Github when you merge the PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

C# 8 nullable reference type support
2 participants