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

feature: Add bytes/strings NewReader support #46

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

spencerschrock
Copy link

@spencerschrock spencerschrock commented Nov 29, 2023

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.

@butuzov
Copy link
Owner

butuzov commented Dec 1, 2023

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

Thank you!

@butuzov
Copy link
Owner

butuzov commented Dec 4, 2023

@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 butuzov self-assigned this Dec 4, 2023
@spencerschrock
Copy link
Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants