Repository navigation
[Broken Build]: Error IDE0040: Accessibility modifiers required #78295
Description
Issue Description
I get Error IDE0040: Accessibility modifiers required on lot's of interfaces in my build overnight on hosted agents, why did you add a breaking changes in a minor upgrade with 9.0.2?
I suspect it's this one: #74245
Steps to Reproduce
The hosted agents yesterday on 9.0.102 works fine. the new builds today fetched dotnet 9.0.200, and fails all over the place.
Expected Behavior
I expect builds not to break on a minor upgrade.
Actual Behavior
Builds break.
Ask us questions
No response
Reacted by Marc Dürst, Benedict Zendel, Alex, ldeluigi and Johannes Mols
Activity
BenjaminBrienen commentedon Feb 12, 2025
Fixing a bug is not a breaking change
baronfel commentedon Feb 12, 2025
@BenjaminBrienen we do consider fixing bugs that will break builds unexpectedly as a breaking change from the perspective of deciding when the change should roll out. Analysis bugs are especially thorny here - we ideally would like updates to new SDK feature bands to not break users, and so if we had applied our principles uniformly there's a real chance that we would have delayed this enforcement to .NET 10 TFMs.
@bj8sk did you have your editorconfig set up to request errors for this diagnostic, or was this a change in default editorconfig configuration that caught you?
bj8sk commentedon Feb 12, 2025
Have this line in .editorconfig for a while without issues until today:
dotnet_diagnostic.IDE0040.severity = error # Accessibility modifiers required
CyrusNajmabadi commentedon Feb 12, 2025
What items are reporting issues where they are missing accessibility modifiers?
CyrusNajmabadi commentedon Feb 12, 2025
Note: These are Roslyn IDE warnings. And we have never had an 'bug fixes cannot be made' when it comes to analyzers. If SDK pulls in our analyzers and has their own bar here, then i think some system will have to happen on the SDK to not update analyzers until an appropriate release happens. :)
jaredpar commentedon Feb 12, 2025
They are also compiler warnings and hence fall into the build diagnostics category.
CyrusNajmabadi commentedon Feb 12, 2025
As mentioned, we've never had a "bug fixes cannot be made" when it comes to analyzers. We've likely fixed thousands of issues over the years which change what analyzers report. Analyzers have always been suggestions from us, and up to users as to how they integrate those suggestions into their experiences. We've genuinely never had a constraint (since the beginning) on being able to update our analyzers in this way.
Note: SDK may have such rules. But our agreement with SDK on consuming us was that we would continue development of our features as we have so far, as we don't want to add limitations and constraints on our mainline scenarios. If SDK has limitations/constraints, i think we'll have to figure out what to do on that end. Perhaps not taking Roslyn IDE analyzer updates, except on certain cadences.
jaredpar commentedon Feb 12, 2025
That is not realistic for many other reasons.
No one has said this constraint exists. @baronfel noted that analyzers, and compilers, can be thorny cases here. When the change is impactful though we give consideration to adding an entry in the breaking changes documentation so customers are aware.
CyrusNajmabadi commentedon Feb 12, 2025
I think we could do that. But it would def be a challenge as every analyzer bugfix seems to apply here. This is because any new suggestion that appears could break if a user has elevated that suggestion to a warning. Similarly, any suggestion that is removed could break a user if they have suppressed that suggestion, and have a warning on for unnecessary suppressions. :)
I'm primarily saying that we've been shipping and changing analyzers constantly (both in Roslyn and Roslyn-Analyzers), and haven't ever been adding such documentation. So while i'm not opposed to discussion on this or going that route, I'm def leaning more to: If we haven't needed this yet, do we really need it now?
Put in other words: what has changed to cause us to feel that we need to adjust our processes now? Thanks!
jaredpar commentedon Feb 12, 2025
This is a case where the change had a non-trivial impact on a real world scenario. At that point it is more likely that it could impact other customers and raises to the level of put in a breaking change notification to increase visibility. This is not a change in process, this is how we've operated for some time.
CyrusNajmabadi commentedon Feb 12, 2025
Could you clarify what distinguishes that from other reports we have in the past where people have reported we're now reporting more and our position has been that that's how IDE analyzers work?
Basically, I'm not seeing what's different here. Or what set of rules i would apply that would cause us to not say anything about all the prior occurrences, but would here?
I think i'm pushing back on that. I could be wrong. But i don't recall any other times when we've done this for IDE analyzers, despite routinely updating/augmenting them in ways that impact codebases.
That we can update our analyzers and cause new suggestions to raised is a virtue that we have taken advantage of numerous times. For example, analyzers detecting broader sets of patterns to be able to help customers refine.
Note: i have no problem with this. My general thought though is simply that the best thing in that case is that every change to analyzers taht impacts their results goes into the breaking changes doc.
jaredpar commentedon Feb 12, 2025
IDE analyzers do not get any sort of special treatment here. They are a part of build that produces diagnostics. No one at that level has the license to change diagnostics as they desire. Instead they need to fall into the existing structure with which we introduce and change existing diagnostics.
In terms of what distinguishes this from other cases: whether to add a breaking change notice is hand wavy. At the moment we're pretty sensitive to breaks and hence erroring on the side of do over do not.
The IDE has made at least four changes in the last 10 months to back out new diagnostics that adversely impacted customer builds. This is not the level of back out but potentially in the level of "add a notice". True the IDE hasn't yet had to add a notice for impactful changes, mostly because they've backed out changes instead, but that doesn't mean the IDE is immune from them.
Every time you do that you're taking a calculated risk.
tj-concentrix commentedon Feb 13, 2025
This is an issue for us too because we have this set to a warning and our pipeline builds set
--warnaserror. So after upgrading to 9.0.2, any interface member which has apublicmodifier is now a warning & our builds are failing.We're using the default value of
for_non_interface_members, which is documented as "Prefer accessibility modifiers except for public interface members."In the past, this has meant "this is not enforced one way or the other for interfaces" which is what we want. But now, the meaning has changed so that they're explicitly not allowed.
So my questions are:
.editorconfigto configure this rule so that it's awarningfor non-interfaces butnonefor interfaces?CyrusNajmabadi commentedon Feb 13, 2025
No. This was the intended way for this to work.
Yup. It was always intended to work this way.
No.
But if you have
for_non_interface_members, then why do you have accessibility modifiers on interfaces? :)If you do want accessibility modifiers on interfaces, tehn you can just set this option to
alwaysinstead.for_non_interface_membersexists precisely for the common .Net coding pattern where everything must have accessibility mods except interface members, since those didn't previously allow modifiers, and have been understood to be public since the start of C# :)tj-concentrix commentedon Feb 13, 2025
We want to leave it up to the developers on whether to add them on interface members. Especially when used with default interface implementations, it can make it more clear.
But for all other types, we want to require them, even if they're the default.
That would break all of our existing code that was written before C# 8. Even if we decided to enforce them for consistency everywhere in new code, it's not feasible for us to make this change globally to all the old code.
26 remaining items
WimLeflere commentedon Sep 24, 2025
Has this issue been fixed? In which version?
With dotnet v9.0.305 is still seems to occur.
CyrusNajmabadi commentedon Sep 24, 2025
@WimLeflere what issue are you referring to? What incorrect behavior are you seeing?
WimLeflere commentedon Sep 24, 2025
I'm still getting error IDE0040 for fields in a public interface.
CyrusNajmabadi commentedon Sep 24, 2025
@WimLeflere what does your editorconfig look like?
WimLeflere commentedon Sep 24, 2025
CyrusNajmabadi commentedon Sep 24, 2025
You have stated that you always require accessibility modifiers. And your
bool IsEnabled { get; set; }member lacks a modifier. So you are getting an error (as per your settings).If you want an exception for interface members, you can say
= for_non_interface_members:error.WimLeflere commentedon Sep 24, 2025
Thanks, that resolves the issue.