Skip to content

Commit eb5d1c7

Browse files
GrahamTheCoderGrahamTheCoder
authored
Merge pull request #1117 from TymurGubayev/fix/PropertySetterComments/1
Don't gather all trivia of a parameterized property in the first accessor
2 parents e65b3fd + e9aefd3 commit eb5d1c7

10 files changed

Lines changed: 85 additions & 52 deletions

File tree

‎CodeConverter/CSharp/DeclarationNodeVisitor.cs‎

Lines changed: 29 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -305,7 +305,10 @@ private MemberDeclarationSyntax[] GetAdditionalDeclarations(VBSyntax.StatementSy
305305
private async Task<MemberDeclarationSyntax> ConvertMemberAsync(VBSyntax.StatementSyntax member)
306306
{
307307
try {
308-
return await member.AcceptAsync<MemberDeclarationSyntax>(TriviaConvertingDeclarationVisitor);
308+
var sourceTriviaMapKind = member is VBSyntax.PropertyBlockSyntax propBlock && ShouldConvertAsParameterizedProperty(propBlock.PropertyStatement)
309+
? SourceTriviaMapKind.SubNodesOnly
310+
: SourceTriviaMapKind.All;
311+
return await member.AcceptAsync<MemberDeclarationSyntax>(TriviaConvertingDeclarationVisitor, sourceTriviaMapKind);
309312
} catch (Exception e) {
310313
return CreateErrorMember(member, e);
311314
}
@@ -674,7 +677,7 @@ public override async Task<CSharpSyntaxNode> VisitPropertyStatement(VBSyntax.Pro
674677
throw new NotImplementedException("MyClass indexing not implemented");
675678
}
676679
var methodDeclarationSyntaxs = await propertyBlock.Accessors.SelectAsync(async a =>
677-
await a.AcceptAsync<MethodDeclarationSyntax>(TriviaConvertingDeclarationVisitor, a == propertyBlock.Accessors.First() ? SourceTriviaMapKind.All : SourceTriviaMapKind.None));
680+
await a.AcceptAsync<MethodDeclarationSyntax>(TriviaConvertingDeclarationVisitor, SourceTriviaMapKind.All));
678681
var accessorMethods = methodDeclarationSyntaxs.Select(WithMergedModifiers).ToArray();
679682

680683
if (hasExplicitInterfaceImplementation) {
@@ -938,7 +941,30 @@ private static AccessorListSyntax ConvertSimpleAccessors(bool isWriteOnly, bool
938941

939942
public override async Task<CSharpSyntaxNode> VisitPropertyBlock(VBSyntax.PropertyBlockSyntax node)
940943
{
941-
return await node.PropertyStatement.AcceptAsync<CSharpSyntaxNode>(TriviaConvertingDeclarationVisitor, SourceTriviaMapKind.SubNodesOnly);
944+
var converted = await node.PropertyStatement.AcceptAsync<CSharpSyntaxNode>(TriviaConvertingDeclarationVisitor, SourceTriviaMapKind.SubNodesOnly);
945+
946+
if (converted is MethodDeclarationSyntax) {
947+
var first = (MethodDeclarationSyntax)converted;
948+
949+
var firstCsConvertedToken = first.GetFirstToken();
950+
var firstVbSourceToken = node.GetFirstToken();
951+
first = first.ReplaceToken(firstCsConvertedToken, firstCsConvertedToken.WithSourceMappingFrom(firstVbSourceToken));
952+
953+
var members = _additionalDeclarations[node];
954+
var last = members.OfType<MethodDeclarationSyntax>().LastOrDefault() ?? first;
955+
var lastIx = members.ToList().IndexOf(last);
956+
var lastIsFirst = lastIx < 0;
957+
var lastCsConvertedToken = last.GetLastToken();
958+
var lastVbSourceToken = node.GetLastToken();
959+
last = last.ReplaceToken(lastCsConvertedToken, lastCsConvertedToken.WithSourceMappingFrom(lastVbSourceToken));
960+
961+
converted = lastIsFirst ? last : first;
962+
if (!lastIsFirst) {
963+
members[lastIx] = last;
964+
}
965+
}
966+
967+
return converted;
942968
}
943969

944970
public override async Task<CSharpSyntaxNode> VisitAccessorBlock(VBSyntax.AccessorBlockSyntax node)

‎Tests/CSharp/MemberTests/MemberTests.cs‎

Lines changed: 1 addition & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -590,11 +590,9 @@ public virtual int get_RenamedPropertyParam(int i)
590590
{
591591
return 1;
592592
}
593-
594593
public virtual void set_RenamedPropertyParam(int i, int value)
595594
{
596595
}
597-
598596
int IClass.get_ReadOnlyPropParam(int i) => get_RenamedPropertyParam(i);
599597
600598
public virtual int RenamedReadOnlyProperty
@@ -614,11 +612,9 @@ public virtual int get_RenamedWriteOnlyPropParam(int i)
614612
{
615613
return 1;
616614
}
617-
618615
public virtual void set_RenamedWriteOnlyPropParam(int i, int value)
619616
{
620617
}
621-
622618
void IClass.set_WriteOnlyPropParam(int i, int value) => set_RenamedWriteOnlyPropParam(i, value);
623619
624620
public virtual int RenamedWriteOnlyProperty
@@ -669,7 +665,7 @@ public void set_Prop(int i, string value)
669665
_Prop_bSet = false;
670666
}
671667
672-
}", incompatibleWithAutomatedCommentTesting: true);// Known bug: Additional declarations don't get comments correctly converted
668+
}");
673669
}
674670

675671
[Fact]
@@ -832,21 +828,18 @@ public string get_ReadOnlyPropRenamed(int i)
832828
{
833829
throw new NotImplementedException();
834830
}
835-
836831
string IClass.get_ReadOnlyPropToRename(int i) => get_ReadOnlyPropRenamed(i);
837832
838833
public virtual void set_WriteOnlyPropRenamed(int i, string value)
839834
{
840835
throw new NotImplementedException();
841836
}
842-
843837
void IClass.set_WriteOnlyPropToRename(int i, string value) => set_WriteOnlyPropRenamed(i, value);
844838
845839
public virtual string get_PropRenamed(int i)
846840
{
847841
throw new NotImplementedException();
848842
}
849-
850843
public virtual void set_PropRenamed(int i, string value)
851844
{
852845
throw new NotImplementedException();
@@ -859,21 +852,18 @@ private string get_ReadOnlyPropNonPublic(int i)
859852
{
860853
throw new NotImplementedException();
861854
}
862-
863855
string IClass.get_ReadOnlyPropNonPublic(int i) => get_ReadOnlyPropNonPublic(i);
864856
865857
protected internal virtual void set_WriteOnlyPropNonPublic(int i, string value)
866858
{
867859
throw new NotImplementedException();
868860
}
869-
870861
void IClass.set_WriteOnlyPropNonPublic(int i, string value) => set_WriteOnlyPropNonPublic(i, value);
871862
872863
internal virtual string get_PropNonPublic(int i)
873864
{
874865
throw new NotImplementedException();
875866
}
876-
877867
internal virtual void set_PropNonPublic(int i, string value)
878868
{
879869
throw new NotImplementedException();
@@ -886,21 +876,18 @@ protected internal virtual string get_ReadOnlyPropRenamedNonPublic(int i)
886876
{
887877
throw new NotImplementedException();
888878
}
889-
890879
string IClass.get_ReadOnlyPropToRenameNonPublic(int i) => get_ReadOnlyPropRenamedNonPublic(i);
891880
892881
private void set_WriteOnlyPropRenamedNonPublic(int i, string value)
893882
{
894883
throw new NotImplementedException();
895884
}
896-
897885
void IClass.set_WriteOnlyPropToRenameNonPublic(int i, string value) => set_WriteOnlyPropRenamedNonPublic(i, value);
898886
899887
internal virtual string get_PropToRenameNonPublic(int i)
900888
{
901889
throw new NotImplementedException();
902890
}
903-
904891
internal virtual void set_PropToRenameNonPublic(int i, string value)
905892
{
906893
throw new NotImplementedException();
@@ -2397,7 +2384,6 @@ private int get_ExplicitProp(string str = """")
23972384
{
23982385
return 5;
23992386
}
2400-
24012387
private void set_ExplicitProp(string str = """", int value = default)
24022388
{
24032389
}
@@ -3072,7 +3058,6 @@ private int get_ExplicitProp(string str)
30723058
{
30733059
return 5;
30743060
}
3075-
30763061
private void set_ExplicitProp(string str, int value)
30773062
{
30783063
}
@@ -3125,7 +3110,6 @@ public virtual int get_PropParams(string str)
31253110
{
31263111
return 5;
31273112
}
3128-
31293113
public virtual void set_PropParams(string str, int value)
31303114
{
31313115
}
@@ -3748,7 +3732,6 @@ private int get_ExplicitProp(string str)
37483732
{
37493733
return 5;
37503734
}
3751-
37523735
private void set_ExplicitProp(string str, int value)
37533736
{
37543737
}

‎Tests/CSharp/MemberTests/PropertyMemberTests.cs‎

Lines changed: 55 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ Return LastName & "" "" & FirstName
8484
Return FirstName & "" "" & LastName
8585
End If
8686
End Get
87-
' Bug: Comment moves inside generated method
87+
' This comment belongs to the set method
8888
Friend Set
8989
If isFirst Then FirstName = Value
9090
End Set
@@ -110,9 +110,8 @@ public string get_FullName(bool lastNameFirst, bool isFirst)
110110
{
111111
return FirstName + "" "" + LastName;
112112
}
113-
// Bug: Comment moves inside generated method
114113
}
115-
114+
// This comment belongs to the set method
116115
internal void set_FullName(bool lastNameFirst, bool isFirst, string value)
117116
{
118117
if (isFirst)
@@ -151,7 +150,6 @@ public float get_SomeProp(int index)
151150
{
152151
return 1.5f;
153152
}
154-
155153
public void set_SomeProp(int index, float value)
156154
{
157155
}
@@ -176,7 +174,7 @@ Public Property FullName(Optional ByVal isFirst As Boolean = False) As String
176174
Get
177175
Return FirstName & "" "" & LastName
178176
End Get
179-
'Bug: Comment moves inside generated get method
177+
'This comment belongs to the set method
180178
Friend Set
181179
If isFirst Then FirstName = Value
182180
End Set
@@ -197,9 +195,8 @@ internal partial class TestClass
197195
public string get_FullName(bool isFirst = false)
198196
{
199197
return FirstName + "" "" + LastName;
200-
// Bug: Comment moves inside generated get method
201198
}
202-
199+
// This comment belongs to the set method
203200
internal void set_FullName(bool isFirst = false, string value = default)
204201
{
205202
if (isFirst)
@@ -259,7 +256,6 @@ public string get_MyProp(int blah)
259256
{
260257
return blah.ToString();
261258
}
262-
263259
public void set_MyProp(int blah, string value)
264260
{
265261
}
@@ -288,6 +284,57 @@ public void ReturnWhatever(MyEnum m)
288284
}");
289285
}
290286

287+
[Fact]
288+
public async Task TestParameterizedPropertyWithTriviaAsync()
289+
{
290+
//issue 1095
291+
await TestConversionVisualBasicToCSharpAsync(
292+
@"Class IndexedPropertyWithTrivia
293+
'a
294+
Property P(i As Integer) As Integer
295+
'b
296+
Get
297+
'1
298+
Dim x = 1 '2
299+
'3
300+
End Get
301+
302+
'c
303+
Set(value As Integer)
304+
'4
305+
Dim x = 1 '5
306+
'6
307+
x = value + i '7
308+
'8
309+
End Set
310+
'd
311+
End Property
312+
End Class", @"
313+
internal partial class IndexedPropertyWithTrivia
314+
{
315+
// a
316+
// b
317+
public int get_P(int i)
318+
{
319+
// 1
320+
int x = 1; // 2
321+
return default;
322+
// 3
323+
}
324+
325+
// c
326+
public void set_P(int i, int value)
327+
{
328+
// 4
329+
int x = 1; // 5
330+
// 6
331+
x = value + i; // 7
332+
// 8
333+
// d
334+
}
335+
}");
336+
}
337+
291338
[Fact]
292339
public async Task PropertyWithMissingTypeDeclarationAsync()//TODO Check object is the inferred type
293340
{
@@ -520,7 +567,6 @@ internal int get_Prop2(int x = 1, int y = 2)
520567
{
521568
return default;
522569
}
523-
524570
internal void set_Prop2(int x = 1, int y = 2, int value = default)
525571
{
526572
}
@@ -610,7 +656,6 @@ internal int get_Prop2(int x = 1, int y = 2, int z = 3)
610656
{
611657
return default;
612658
}
613-
614659
internal void set_Prop2(int x = 1, int y = 2, int z = 3, int value = default)
615660
{
616661
}

‎Tests/TestData/MultiFileCharacterization/VBToCSResults/ConvertVbLibraryOnly/VbLibrary/My Project/MyNamespace.Static.3.Designer.cs‎

Lines changed: 0 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎Tests/TestData/MultiFileCharacterization/VBToCSResults/ConvertWholeSolution/ConsoleApp1/My Project/MyNamespace.Static.2.Designer.cs‎

Lines changed: 0 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎Tests/TestData/MultiFileCharacterization/VBToCSResults/ConvertWholeSolution/ConsoleApp4/My Project/MyNamespace.Static.2.Designer.cs‎

Lines changed: 0 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)