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.

Sign up for GitHub

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

Jump to bottom

Don't gather all trivia of a parameterized property in the first accessor #1117

Merged
merged 9 commits into from
Jul 25, 2024
Merged

Don't gather all trivia of a parameterized property in the first accessor #1117

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
4c5d449
add a test
Jul 19, 2024
9e5e6c9
place trivia in all accessors when converting parameterized properties
Jul 19, 2024
0d12fe5
Merge branch 'master' into fix/PropertySetterComments/1
Jul 23, 2024
f776453
adjust the test
Jul 25, 2024
266e7e3
implement special handling for indexed properties' trivia
Jul 25, 2024
a64e208
move the new test to the PropertyMemberTests
Jul 25, 2024
a8c6d10
adjust tests: some old known bugs are fixed
Jul 25, 2024
363a25d
adjust tests: no empty line between getter and setter methods anymore
Jul 25, 2024
e9aefd3
clarify code a tiny bit
Jul 25, 2024
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
32 changes: 29 additions & 3 deletions CodeConverter/CSharp/DeclarationNodeVisitor.cs
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop

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
Open in desktop

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
Open in desktop

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

Loading
Loading