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

Generics are not translated correctly #1077

Closed
mikes-gh opened this issue Jan 31, 2021 · 14 comments
Closed

Generics are not translated correctly #1077

mikes-gh opened this issue Jan 31, 2021 · 14 comments
Labels
enhancement General enhancement request stale tenet-reporters Issue related to coverage output files(reporters)

Comments

@mikes-gh
Copy link
Contributor

mikes-gh commented Jan 31, 2021

It appear generics are not translated correctly in the coverage report. They contain the backtick which would be what it would be if you just did Type.Name

So for example Converter<T> will incorrectly translate to Converter`1 in the report.

See this for an explanation

https://stackoverflow.com/questions/17480990/get-name-of-generic-class-without-tilde

It would make reading the reports easier

@mikes-gh mikes-gh reopened this Jan 31, 2021
@MarcoRossignoli MarcoRossignoli added the waiting for customer Waiting for customer action label Feb 6, 2021
@MarcoRossignoli
Copy link
Collaborator

MarcoRossignoli commented Feb 6, 2021

Which report generator are you using?
Can you attach an image?

@mikes-gh
Copy link
Contributor Author

mikes-gh commented Feb 7, 2021

coverage.cobertura.xml
Here the Generic is MudBaseInput<T> but it shows as MudBaseInput`1

 <class name="MudBlazor.MudBaseInput`1" filename="Base/MudBaseInput.cs" line-rate="0.9031999999999999" branch-rate="1" complexity="25">
          <methods>
            <method name="UpdateTextPropertyAsync" signature="(System.Boolean)" line-rate="1" branch-rate="1" complexity="1">
              <lines>

@MarcoRossignoli
Copy link
Collaborator

MarcoRossignoli commented Feb 7, 2021

Usually the reports are not intended to be manually read, but as input for reports generator tool like https://danielpalme.github.io/ReportGenerator/
The name of object/members are simple standard ToString() that these tool convert accordingly.

Another sample, opencover report https://github.com/OpenCover/opencover/blob/master/samplexml/opencovertests.xml#L252

@MarcoRossignoli MarcoRossignoli added as-designed Expected behaviour and removed waiting for customer Waiting for customer action labels Feb 7, 2021
@mikes-gh
Copy link
Contributor Author

mikes-gh commented Feb 9, 2021

@MarcoRossignol I use Report generator and the reports show it wrong as well because you have lost the original information.

Please see here how to correctly extract the type parameter names for generic types.
https://stackoverflow.com/questions/2448800/given-a-type-instance-how-to-get-generic-type-name-in-c

@mikes-gh
Copy link
Contributor Author

mikes-gh commented Feb 9, 2021

Screenshot 2021-02-09 at 21 28 23

This is the output from report generator you mentioned. You can see it is Hard to tell the generic types apart.

@mikes-gh
Copy link
Contributor Author

mikes-gh commented Feb 9, 2021

Maybe you would accept a PR for this?

@MarcoRossignoli
Copy link
Collaborator

Oh I see what you mean, I apologize.

Need some help from my friend @danielpalme 😄 , Daniel do you have any concern about generics signature update in reports?
You've more experience than me on reports and maybe you can told us if makes sense update or if it's better keep generic name generation as-is.
Do you think it could break something on reporters side?

@MarcoRossignoli MarcoRossignoli added enhancement General enhancement request tenet-reporters Issue related to coverage output files(reporters) and removed as-designed Expected behaviour labels Feb 10, 2021
@mikes-gh
Copy link
Contributor Author

mikes-gh commented Feb 10, 2021

@MarcoRossignoli @danielpalme you will need to encode the angle brackets so they don't break the xml.

&lt; &gt;

edit need to check the cobertura spec

@danielpalme
Copy link

Changing the XML should work. As @mikes-gh mentioned, brackets have to be encoded.

But I would suggest, that I change the behavior in ReportGenerator. That way it would work for all report types, not just for Coberatura files.

I could add a mapping that converts class names like this:

`1 -> <T> 
`2 -> <T1, T2> 
`3 -> <T1, T2, T3>

@mikes-gh
Copy link
Contributor Author

mikes-gh commented Feb 10, 2021

@danielpalme That would be great thank you.
@MarcoRossignoli If we could also support the original signature with maybe a optin switch I will happily PR if you like.
It would be based around the function above.
So for example I could have

Converter<T, U>. as in my code. It just makes it easier to refer to the original method.

danielpalme added a commit to danielpalme/ReportGenerator that referenced this issue Feb 10, 2021
@danielpalme
Copy link

I just made the necessary code changes in ReportGenerator. Next release will contain the change.

@mikes-gh
Copy link
Contributor Author

mikes-gh commented Feb 11, 2021

@danielpalme Thank you. 🙏 I presume if @MarcoRossignoli provides the original generic type names in the source with &gt; &lt; your report will be fine with that too?

@github-actions
Copy link

This issue is stale because it has been open for 3 months with no activity.

@github-actions github-actions bot added the stale label Sep 24, 2023
Copy link

This issue was closed because it has been inactive for 9 months since being marked as stale.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Jun 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement General enhancement request stale tenet-reporters Issue related to coverage output files(reporters)
Projects
None yet
Development

No branches or pull requests

3 participants