mirror icon indicating copy to clipboard operation
mirror copied to clipboard

feature: Add bytes/strings NewReader support

Open spencerschrock opened this issue 1 year ago • 3 comments

Saw a few occurrences of this in my code, and noticed the linter wasn't picking them up.

r := strings.NewReader(string(content))

MIRROR_FUNCS.md was edited manually, but the tests were generated with make test-generate.

spencerschrock avatar Nov 29 '23 20:11 spencerschrock

I like it. Let me check what Ican do about having MIRROR_FUNCS.md generated and i will merge you PR.

Thank you!

butuzov avatar Dec 01 '23 08:12 butuzov

@spencerschrock thank you for yout PR, but for now lets put it onhold. While i would go for using correct struct/function both functions will return different types (yeah, same interface but types are different). So it require a bit different implementation. I will work with your code and no change required from your side.

However it most probably will take some time on my side, as I am now not in friendly conditions of home office.

cheers.

butuzov avatar Dec 04 '23 08:12 butuzov

While i would go for using correct struct/function both functions will return different types (yeah, same interface but types are different). So it require a bit different implementation. I will work with your code and no change required from your side.

Ah shoot, I glanced over that. I just saw the Reader type and forgot one is strings.Reader and one is bytes.Reader. I can see how this would be problematic depending on how the resulting type is assigned/used.

This sort of problem has prevented me from removing other string/[]byte conversions in my code. For example, where I'm unable to change to r.Find(line) below because I need the result to be a string vs []byte.

var found string = r.FindString(string(line))

I'm new to writing/patching static analysis tools, so I'm excited to see your future changes.

However it most probably will take some time on my side, as I am now not in friendly conditions of home office.

Of course, take as long as you need. Thanks for taking a look at the PR in the first place, and of course stay safe.

spencerschrock avatar Dec 05 '23 20:12 spencerschrock