Merge pull request #425 from mrmonday/422-constructor-visibility

Ensure class constructors are marked as public [#422]

CHANGELOG.md

@@ -20,6 +20,7 @@ All notable changes to the code converter will be documented here.
 * Correctly handle type promoted module symbols (#375)
 * Prefer renamed imports for name resolution (#401)
 * Correctly convert ambiguous names (#332)
+* Ensure correct visibility for constructors (#422)
 
 ### C# -> VB
 * Convert property accessors with visiblity modifiers (#92)

ICSharpCode.CodeConverter/CSharp/CommonConversions.cs

@@ -259,14 +259,14 @@ private static SyntaxToken CsEscapedIdentifier(string text)
         }
 
         public SyntaxTokenList ConvertModifiers(SyntaxNode node, IEnumerable<SyntaxToken> modifiers,
-            TokenContext context = TokenContext.Global, bool isVariableOrConst = false, bool isConstructor = false, params CSSyntaxKind[] extraCsModifierKinds)
+            TokenContext context = TokenContext.Global, bool isVariableOrConst = false, params CSSyntaxKind[] extraCsModifierKinds)
         {
             ISymbol declaredSymbol = _semanticModel.GetDeclaredSymbol(node);
             var declaredAccessibility = declaredSymbol?.DeclaredAccessibility ?? Accessibility.NotApplicable;
 
             var contextsWithIdenticalDefaults = new[] { TokenContext.Global, TokenContext.Local, TokenContext.InterfaceOrModule, TokenContext.MemberInInterface };
             bool isPartial = declaredSymbol.IsPartialClassDefinition() || declaredSymbol.IsPartialMethodDefinition() || declaredSymbol.IsPartialMethodImplementation();
-            bool implicitVisibility = contextsWithIdenticalDefaults.Contains(context) || isVariableOrConst || isConstructor;
+            bool implicitVisibility = contextsWithIdenticalDefaults.Contains(context) || isVariableOrConst || declaredSymbol.IsStaticConstructor();
             if (implicitVisibility && !isPartial) declaredAccessibility = Accessibility.NotApplicable;
             var modifierSyntaxs = ConvertModifiersCore(declaredAccessibility, modifiers, context)
                 .Concat(extraCsModifierKinds.Select(SyntaxFactory.Token))

ICSharpCode.CodeConverter/CSharp/DeclarationNodeVisitor.cs

@@ -1041,7 +1041,7 @@ public override async Task<CSharpSyntaxNode> VisitConstructorBlock(VBSyntax.Cons
         {
             var block = node.BlockStatement;
             var attributes = await block.AttributeLists.SelectManyAsync(_expressionNodeVisitor.ConvertAttribute);
-            var modifiers = CommonConversions.ConvertModifiers(block, block.Modifiers, GetMemberContext(node), isConstructor: true);
+            var modifiers = CommonConversions.ConvertModifiers(block, block.Modifiers, GetMemberContext(node));
 
             var ctor = (node.Statements.FirstOrDefault() as VBSyntax.ExpressionStatementSyntax)?.Expression as VBSyntax.InvocationExpressionSyntax;
             var ctorExpression = ctor?.Expression as VBSyntax.MemberAccessExpressionSyntax;

Tests/CSharp/ExpressionTests.cs

@@ -977,7 +977,7 @@ End Get
     End Property
 End Class", @"public partial class Class1
 {
-    Class1()
+    public Class1()
     {
         var needsInitialization = default(int);
         int notUsed;

Tests/CSharp/MemberTests.cs

@@ -36,6 +36,20 @@ await TestConversionVisualBasicToCSharp(
 }");
         }
 
+        [Fact]
+        public async Task TestConstructorVisibility()
+        {
+            await TestConversionVisualBasicToCSharpWithoutComments(@"Class Class1
+    Sub New(x As Boolean)
+    End Sub
+End Class", @"internal partial class Class1
+{
+    public Class1(bool x)
+    {
+    }
+}");
+        }
+
         [Fact]
         public async Task TestModuleConstructor()
         {
@@ -779,7 +793,7 @@ public async Task TestConstructorWithImplicitPublicAccessibility()
         {
             await TestConversionVisualBasicToCSharp(
 @"Sub New()
-End Sub", @"SurroundingClass()
+End Sub", @"public SurroundingClass()
 {
 }");
         }