Runtime: Tip: add string.Contains(string, StringComparison)

[nopara73]

Add convenience API 'string.Contains' overload with StringComparison argument.
Motivation: 2000+ votes on StackOverflow

c# partial class String { //public bool Contains(string value); // Exists public bool Contains(string value, StringComparison comparisonType); }

Original post

I am not sure if this topic has came up before or not, but it looks to me a low hangig fruit with great demand: https://stackoverflow.com/a/444818/2061103

[karelz]

Sounds reasonable to me -- top post updated with formal API proposal

[khellang]

The MSDN documentation for string.Contains even has a remark on how to write this method yourself, as an extension method. It's obviously useful.

[karelz]

API review: Approved as described above.
Marking as up-for-grabs, should be fairly straightforward to do. Any takers?

This is tip of the iceberg. We have larger inconsistencies in string APIs. It would be great if someone wants to do a sweep and list all things we should add for consistency - should be tracked in issue dotnet/runtime#14065. Anyone interested?

[karelz]

Thanks for submitting the PR @yvanin! I sent you invite to CoreFX and CoreCLR repos, so that we can assign issues to you when you grab them. Please ping me when you accept the invitations.
Temporarily assigning to myself.

cc @joperezr @AlexGhiondea

[yvanin]

@karelz, I accepted the invitations and can grab this issue, meanwhile waiting for System.Private.CoreLib to be published

[danmosemsft]

@yvanin " meanwhile waiting for System.Private.CoreLib to be published" ... not sure what you mean?

The work would be done in the coreclr repo, where String.cs lives, and also exposed in corefx\src\System.Runtime\ref\System.Runtime.cs and tests somewhere in corefx\src\System.Runtimetests\System. We ask that both PR's be up and reviewed before we merge the coreclr one, so we don't forget tests. Then when coreclr reaches corefx a day or so later, we merge corefx.

[danmosemsft]

Not sure if you've done this before -- don't hesitate to ask if you have questions/issues.

[yvanin]

@danmosemsft I see, I've never done this before and was using this doc as a guide.
It makes sense though, I will start working on the test suite.

And to confirm, I can add the new method in corefx\src\System.Runtime\ref\System.Runtime.cs directly, right? This file is not autogenerated from coreclr releases, is it?

[karelz]

@yvanin I would suggest to start with new CoreFX contributor docs (linked from CoreFX main page).
While they are under construction, we are slowly improving them and if some things are not covered, we want to add them (with your help as you can edit the wiki, or directly by us). There is also gitter room where people can get help.

cc @ianhays

[danmosemsft]

@yvanin it's not autogenerated. It looks that way because it was originally. It defines the surface area you can reference, which generally corresponds closely to what's public/protected in the implementation.

[yvanin]

@danmosemsft how can I keep the tests from not failing when the referenced API method is not there yet?

[danmosemsft]

@yvanin in CI? You can't -- when CoreFX gets the Coreclr change, we rerun the tests.
Locally, as you work on your CoreCLr change? See here: https://github.com/dotnet/corefx/blob/7059f8b316723f5db64e4b1d28e2b1e8b137ab1e/Documentation/project-docs/developer-guide.md#testing-with-private-coreclr-bits

[danmosemsft]

Now the coreCLr change is merged, within a few hours there will normally be a PR with a title like this one https://github.com/dotnet/corefx/pull/21221 in the Corefx repo. When that is merged, it updates a reference in this repo to point to a CoreCLR with your change. Then we can re-run tests on your PR that exposes and tests the API. When that is green and folks are signed off, we commit it. then you are done.

[danmosemsft]

Actually I take that back -- please also port the implementation into https://github.com/dotnet/corert -- we cannot merge the tests until that is also done, as in some configurations those same tests basically run against corert.