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

Adding ioperation tests for ForEachStatement #21048

Merged
merged 5 commits into from
Jul 29, 2017

Conversation

jinujoseph
Copy link
Contributor

@jinujoseph jinujoseph commented Jul 22, 2017

Part of #17602
cc @dotnet/analyzer-ioperation , @cston

@jinujoseph jinujoseph added the PR For Personal Review Only The PR doesn’t require anyone other than the developer to review it. label Jul 22, 2017
@jinujoseph jinujoseph added Test Test failures in roslyn-CI Area-Analyzers and removed PR For Personal Review Only The PR doesn’t require anyone other than the developer to review it. labels Jul 24, 2017
}

[Fact, WorkItem(17602, "https://github.com/dotnet/roslyn/issues/17602")]
public void IForEachLoopStatement_WithYeild()
Copy link
Member

Choose a reason for hiding this comment

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

Typo: Yield

}

[Fact, WorkItem(17602, "https://github.com/dotnet/roslyn/issues/17602")]
public void IForEachLoopStatement_Struct()
Copy link
Member

Choose a reason for hiding this comment

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

Is this equivalent to the earlier Array tests? If so, consider removing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

}

[Fact, WorkItem(17602, "https://github.com/dotnet/roslyn/issues/17602")]
public void IForEachLoopStatement_ConstantNull()
Copy link
Member

Choose a reason for hiding this comment

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

Consider renaming since the string is not null. Perhaps just IForEachLoopStatement_String.

}

[Fact, WorkItem(17602, "https://github.com/dotnet/roslyn/issues/17602")]
public void IForEachLoopStatement_ForEachOutVar()
Copy link
Member

Choose a reason for hiding this comment

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

Does out var i affect the IForEachLoopStatement? If not, consider removing the test.

public static implicit operator C(string s)
{
return new C();
}
Copy link
Member

Choose a reason for hiding this comment

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

The conversion operator is unused.

}

[Fact, WorkItem(17602, "https://github.com/dotnet/roslyn/issues/17602")]
public void IForEachLoopStatement_Withhrow()
Copy link
Member

Choose a reason for hiding this comment

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

Typo: WithThrow

Class Program
Public Shared Sub Main()
Const s As String = Nothing
For Each y In TryCast(s, String)'BIND:"For Each y In TryCast(s, String)"
Copy link
Member

Choose a reason for hiding this comment

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

TryCast is not necessary.

Return New Integer() {1, 2, 3}.GetEnumerator()
End Function
End Structure
]]>.Value
Copy link
Member

Choose a reason for hiding this comment

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

Code above does not include a query.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

End Sub

<Fact(), WorkItem(17602, "https://github.com/dotnet/roslyn/issues/17602")>
Public Sub IForEachLoopStatement_InvalidConverstion()
Copy link
Member

Choose a reason for hiding this comment

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

Typo: Conversion

@jinujoseph jinujoseph force-pushed the ForEachTests branch 2 times, most recently from 3c62d20 to 1d061b9 Compare July 25, 2017 16:59
}

[Fact, WorkItem(17602, "https://github.com/dotnet/roslyn/issues/17602")]
public void IForEachLoopStatement_WithYield()
Copy link
Member

Choose a reason for hiding this comment

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

yield only affects the implementation of IEnumerable<T>, not the foreach. Consider removing test or replacing IForEachLoopStatement_WithList since both are testing IEnumerable<T>.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure

}

[Fact, WorkItem(17602, "https://github.com/dotnet/roslyn/issues/17602")]
public void IForEachLoopStatement_Nested()
Copy link
Member

Choose a reason for hiding this comment

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

IForEachLoopStatement does not depend on the content of Body so the inner foreach should not affect the outer foreach. Consider removing the tests of nested foreach.

End Sub

<Fact(), WorkItem(17602, "https://github.com/dotnet/roslyn/issues/17602")>
Public Sub IForEachLoopStatement_Nested()
Copy link
Member

Choose a reason for hiding this comment

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

Consider removing nested For Each tests since the statements in the Body should not affect the IForEachLoopStatement.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

End Sub

<Fact(), WorkItem(17602, "https://github.com/dotnet/roslyn/issues/17602")>
Public Sub IForEachLoopStatement_FuncCall()
Copy link
Member

Choose a reason for hiding this comment

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

_FuncCall test should be covered by other array tests.

@cston
Copy link
Member

cston commented Jul 26, 2017

LGTM

@jinujoseph
Copy link
Contributor Author

test windows_debug_unit32_prtest

@jinujoseph
Copy link
Contributor Author

test windows_release_vs-integration_prtest

@jinujoseph jinujoseph merged commit fdb83fa into dotnet:features/ioperation Jul 29, 2017
333fred added a commit to 333fred/roslyn that referenced this pull request Aug 4, 2017
…sion-expression-rewrite

* dotnet/features/ioperation: (229 commits)
  Adding Ioperation tests for WhileUntilLoopStatement (dotnet#21047)
  marked assert that needs to be re-enabled
  addressing more PR feedbacks
  PR feedback
  Remove InvocationReasons enum boxing
  PR feedbacks
  Expose if a Binary/Unary operator was 'Lifted' at the IExpression level. (dotnet#14779)
  addressing PR feedback
  added comments
  Update VS Integration machines to 15.3 Preview 6 (dotnet#21240)
  fixed typo
  Fixed dotnet#18763 Compiler crash on bad code in the IDE (dotnet#20903)
  Fix typo in ERR_RefReturnParameter2 (dotnet#21235)
  Fix unbound recursion with const var field in script (dotnet#21223)
  Typo fix (dotnet#20513)
  PR feedbacks and added some more tests
  When producing character literals, surrogate characters should be escaped. (dotnet#20720)
  Fix build correctness issues
  Fix possible null reference warnings
  Adding ioperation tests for ForEachStatement (dotnet#21048)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants