Skip to content

Commit

Permalink
Merge pull request #1117 from TymurGubayev/fix/PropertySetterComments/1
Browse files Browse the repository at this point in the history 
Don't gather all trivia of a parameterized property in the first accessor
  • Loading branch information
@GrahamTheCoder
GrahamTheCoder authored Jul 25, 2024
2 parents e65b3fd + e9aefd3 commit eb5d1c7
Show file tree
Hide file tree
Showing 10 changed files with 85 additions and 52 deletions.
32 changes: 29 additions & 3 deletions CodeConverter/CSharp/DeclarationNodeVisitor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -305,7 +305,10 @@ private MemberDeclarationSyntax[] GetAdditionalDeclarations(VBSyntax.StatementSy
private async Task<MemberDeclarationSyntax> ConvertMemberAsync(VBSyntax.StatementSyntax member)
{
try {
return await member.AcceptAsync<MemberDeclarationSyntax>(TriviaConvertingDeclarationVisitor);
var sourceTriviaMapKind = member is VBSyntax.PropertyBlockSyntax propBlock && ShouldConvertAsParameterizedProperty(propBlock.PropertyStatement)
? SourceTriviaMapKind.SubNodesOnly
: SourceTriviaMapKind.All;
return await member.AcceptAsync<MemberDeclarationSyntax>(TriviaConvertingDeclarationVisitor, sourceTriviaMapKind);
} catch (Exception e) {
return CreateErrorMember(member, e);
}
Expand Down Expand Up @@ -674,7 +677,7 @@ public override async Task<CSharpSyntaxNode> VisitPropertyStatement(VBSyntax.Pro
throw new NotImplementedException("MyClass indexing not implemented");
}
var methodDeclarationSyntaxs = await propertyBlock.Accessors.SelectAsync(async a =>
await a.AcceptAsync<MethodDeclarationSyntax>(TriviaConvertingDeclarationVisitor, a == propertyBlock.Accessors.First() ? SourceTriviaMapKind.All : SourceTriviaMapKind.None));
await a.AcceptAsync<MethodDeclarationSyntax>(TriviaConvertingDeclarationVisitor, SourceTriviaMapKind.All));
var accessorMethods = methodDeclarationSyntaxs.Select(WithMergedModifiers).ToArray();

if (hasExplicitInterfaceImplementation) {
Expand Down Expand Up @@ -938,7 +941,30 @@ private static AccessorListSyntax ConvertSimpleAccessors(bool isWriteOnly, bool

public override async Task<CSharpSyntaxNode> VisitPropertyBlock(VBSyntax.PropertyBlockSyntax node)
{
return await node.PropertyStatement.AcceptAsync<CSharpSyntaxNode>(TriviaConvertingDeclarationVisitor, SourceTriviaMapKind.SubNodesOnly);
var converted = await node.PropertyStatement.AcceptAsync<CSharpSyntaxNode>(TriviaConvertingDeclarationVisitor, SourceTriviaMapKind.SubNodesOnly);

if (converted is MethodDeclarationSyntax) {
var first = (MethodDeclarationSyntax)converted;

var firstCsConvertedToken = first.GetFirstToken();
var firstVbSourceToken = node.GetFirstToken();
first = first.ReplaceToken(firstCsConvertedToken, firstCsConvertedToken.WithSourceMappingFrom(firstVbSourceToken));

var members = _additionalDeclarations[node];
var last = members.OfType<MethodDeclarationSyntax>().LastOrDefault() ?? first;
var lastIx = members.ToList().IndexOf(last);
var lastIsFirst = lastIx < 0;
var lastCsConvertedToken = last.GetLastToken();
var lastVbSourceToken = node.GetLastToken();
last = last.ReplaceToken(lastCsConvertedToken, lastCsConvertedToken.WithSourceMappingFrom(lastVbSourceToken));

converted = lastIsFirst ? last : first;
if (!lastIsFirst) {
members[lastIx] = last;
}
}

return converted;
}

public override async Task<CSharpSyntaxNode> VisitAccessorBlock(VBSyntax.AccessorBlockSyntax node)
Expand Down
19 changes: 1 addition & 18 deletions Tests/CSharp/MemberTests/MemberTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -590,11 +590,9 @@ public virtual int get_RenamedPropertyParam(int i)
{
return 1;
}
public virtual void set_RenamedPropertyParam(int i, int value)
{
}
int IClass.get_ReadOnlyPropParam(int i) => get_RenamedPropertyParam(i);
public virtual int RenamedReadOnlyProperty
Expand All @@ -614,11 +612,9 @@ public virtual int get_RenamedWriteOnlyPropParam(int i)
{
return 1;
}
public virtual void set_RenamedWriteOnlyPropParam(int i, int value)
{
}
void IClass.set_WriteOnlyPropParam(int i, int value) => set_RenamedWriteOnlyPropParam(i, value);
public virtual int RenamedWriteOnlyProperty
Expand Down Expand Up @@ -669,7 +665,7 @@ public void set_Prop(int i, string value)
_Prop_bSet = false;
}
}", incompatibleWithAutomatedCommentTesting: true);// Known bug: Additional declarations don't get comments correctly converted
}");
}

[Fact]
Expand Down Expand Up @@ -832,21 +828,18 @@ public string get_ReadOnlyPropRenamed(int i)
{
throw new NotImplementedException();
}
string IClass.get_ReadOnlyPropToRename(int i) => get_ReadOnlyPropRenamed(i);
public virtual void set_WriteOnlyPropRenamed(int i, string value)
{
throw new NotImplementedException();
}
void IClass.set_WriteOnlyPropToRename(int i, string value) => set_WriteOnlyPropRenamed(i, value);
public virtual string get_PropRenamed(int i)
{
throw new NotImplementedException();
}
public virtual void set_PropRenamed(int i, string value)
{
throw new NotImplementedException();
Expand All @@ -859,21 +852,18 @@ private string get_ReadOnlyPropNonPublic(int i)
{
throw new NotImplementedException();
}
string IClass.get_ReadOnlyPropNonPublic(int i) => get_ReadOnlyPropNonPublic(i);
protected internal virtual void set_WriteOnlyPropNonPublic(int i, string value)
{
throw new NotImplementedException();
}
void IClass.set_WriteOnlyPropNonPublic(int i, string value) => set_WriteOnlyPropNonPublic(i, value);
internal virtual string get_PropNonPublic(int i)
{
throw new NotImplementedException();
}
internal virtual void set_PropNonPublic(int i, string value)
{
throw new NotImplementedException();
Expand All @@ -886,21 +876,18 @@ protected internal virtual string get_ReadOnlyPropRenamedNonPublic(int i)
{
throw new NotImplementedException();
}
string IClass.get_ReadOnlyPropToRenameNonPublic(int i) => get_ReadOnlyPropRenamedNonPublic(i);
private void set_WriteOnlyPropRenamedNonPublic(int i, string value)
{
throw new NotImplementedException();
}
void IClass.set_WriteOnlyPropToRenameNonPublic(int i, string value) => set_WriteOnlyPropRenamedNonPublic(i, value);
internal virtual string get_PropToRenameNonPublic(int i)
{
throw new NotImplementedException();
}
internal virtual void set_PropToRenameNonPublic(int i, string value)
{
throw new NotImplementedException();
Expand Down Expand Up @@ -2397,7 +2384,6 @@ private int get_ExplicitProp(string str = """")
{
return 5;
}
private void set_ExplicitProp(string str = """", int value = default)
{
}
Expand Down Expand Up @@ -3072,7 +3058,6 @@ private int get_ExplicitProp(string str)
{
return 5;
}
private void set_ExplicitProp(string str, int value)
{
}
Expand Down Expand Up @@ -3125,7 +3110,6 @@ public virtual int get_PropParams(string str)
{
return 5;
}
public virtual void set_PropParams(string str, int value)
{
}
Expand Down Expand Up @@ -3748,7 +3732,6 @@ private int get_ExplicitProp(string str)
{
return 5;
}
private void set_ExplicitProp(string str, int value)
{
}
Expand Down
65 changes: 55 additions & 10 deletions Tests/CSharp/MemberTests/PropertyMemberTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ Return LastName & "" "" & FirstName
Return FirstName & "" "" & LastName
End If
End Get
' Bug: Comment moves inside generated method
' This comment belongs to the set method
Friend Set
If isFirst Then FirstName = Value
End Set
Expand All @@ -110,9 +110,8 @@ public string get_FullName(bool lastNameFirst, bool isFirst)
{
return FirstName + "" "" + LastName;
}
// Bug: Comment moves inside generated method
}
// This comment belongs to the set method
internal void set_FullName(bool lastNameFirst, bool isFirst, string value)
{
if (isFirst)
Expand Down Expand Up @@ -151,7 +150,6 @@ public float get_SomeProp(int index)
{
return 1.5f;
}
public void set_SomeProp(int index, float value)
{
}
Expand All @@ -176,7 +174,7 @@ Public Property FullName(Optional ByVal isFirst As Boolean = False) As String
Get
Return FirstName & "" "" & LastName
End Get
'Bug: Comment moves inside generated get method
'This comment belongs to the set method
Friend Set
If isFirst Then FirstName = Value
End Set
Expand All @@ -197,9 +195,8 @@ internal partial class TestClass
public string get_FullName(bool isFirst = false)
{
return FirstName + "" "" + LastName;
// Bug: Comment moves inside generated get method
}
// This comment belongs to the set method
internal void set_FullName(bool isFirst = false, string value = default)
{
if (isFirst)
Expand Down Expand Up @@ -259,7 +256,6 @@ public string get_MyProp(int blah)
{
return blah.ToString();
}
public void set_MyProp(int blah, string value)
{
}
Expand Down Expand Up @@ -288,6 +284,57 @@ public void ReturnWhatever(MyEnum m)
}");
}

[Fact]
public async Task TestParameterizedPropertyWithTriviaAsync()
{
//issue 1095
await TestConversionVisualBasicToCSharpAsync(
@"Class IndexedPropertyWithTrivia
'a
Property P(i As Integer) As Integer
'b
Get
'1
Dim x = 1 '2
'3
End Get
'c
Set(value As Integer)
'4
Dim x = 1 '5
'6
x = value + i '7
'8
End Set
'd
End Property
End Class", @"
internal partial class IndexedPropertyWithTrivia
{
// a
// b
public int get_P(int i)
{
// 1
int x = 1; // 2
return default;
// 3
}
// c
public void set_P(int i, int value)
{
// 4
int x = 1; // 5
// 6
x = value + i; // 7
// 8
// d
}
}");
}

[Fact]
public async Task PropertyWithMissingTypeDeclarationAsync()//TODO Check object is the inferred type
{
Expand Down Expand Up @@ -520,7 +567,6 @@ internal int get_Prop2(int x = 1, int y = 2)
{
return default;
}
internal void set_Prop2(int x = 1, int y = 2, int value = default)
{
}
Expand Down Expand Up @@ -610,7 +656,6 @@ internal int get_Prop2(int x = 1, int y = 2, int z = 3)
{
return default;
}
internal void set_Prop2(int x = 1, int y = 2, int z = 3, int value = default)
{
}
Expand Down
3 changes: 0 additions & 3 deletions .../VBToCSResults/ConvertVbLibraryOnly/VbLibrary/My Project/MyNamespace.Static.3.Designer.cs
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 0 additions & 3 deletions ...BToCSResults/ConvertWholeSolution/ConsoleApp1/My Project/MyNamespace.Static.2.Designer.cs
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 0 additions & 3 deletions ...BToCSResults/ConvertWholeSolution/ConsoleApp4/My Project/MyNamespace.Static.2.Designer.cs
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit eb5d1c7

Please sign in to comment.