-
Notifications
You must be signed in to change notification settings - Fork 226
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
Fix S1144: Nested type constructor accessibility is wrong in the rule message #7774
Comments
Thank you @neman for the report. The message is indeed wrong. Regarding the second issue:
Do you use assembly scanning via var configuration = new MapperConfiguration(cfg => cfg.AddMaps(myAssembly)); If so, there is nothing we can do about it at the moment. Automapper uses reflection at runtime to find the type and create an instance of it. There are no compile-time dependencies on the constructor. If you are using .Net 5 or above, we could consider adding support for the DynamicallyAccessedMembersAttribute that is used by e.g. the trimmer for such cases: [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.NonPublicNestedTypes)]
public class TagDto
{
public int Id { get; set; }
private class Mapping : Profile
{
public Mapping()
{
CreateMap<Tag, TagDto>();
}
}
} Would you apply this attribute to your Dtos to help our analyzer? If so, I would create a new issue to add support for that attribute. |
I can try to add that attribute, although I do not like to add to my code more than required for the implementation. Is it possible to exclude some classes from analyzer (I am using SonarCloud) e.g. using asterisk and create some rule to exclude all *Dto from this concrete rule? |
This will not work because we do not support that attribute yet. It was a solution proposal to allow you to deal with such a problem in the future. I agree with you here: adding the attribute to silence a tool isn't ideal. That's why I was asking if you would add such an attribute if we add support for it. Given your feedback, I will not create an issue as it seems not worth the effort. I think your proposed solution to exclude the files from the analysis is indeed the better approach. You can do so by specifying |
The constructor's effective visibility is |
We and Roslyn call it EffectiveAccessibility
Given that I would say "Remove this effectively private constructor" if the modifier does not match the effective accessibility. |
The documentation for DynamicallyAccessedMembersAttribute states
Adding this attribute is more than just pleasing a third-party tool. It indicates that this seemingly unused class is accessed via reflection at runtime. It therefore also serves documentation purposes to any code reader and satisfies the "clear" clean-code-attribute.
|
I think you forgot the "unused" word, so it will be something like: |
Description
The message I got in VS IDE is:
This is not private ctor, but public.
And also this private class is used.
Repro steps
This is the code example
And this is used as
.ProjectTo<TagDto>(_mapper.ConfigurationProvider),
Related information
The text was updated successfully, but these errors were encountered: