Skip to content

Conversation

@Youssef1313
Copy link
Member

@Youssef1313 Youssef1313 commented Jul 29, 2025

Follow-up to #6148

@Youssef1313
Copy link
Member Author

Very interesting this is failing despite us setting dotnet_public_api_analyzer.require_api_files = true.

@Youssef1313
Copy link
Member Author

Youssef1313 commented Jul 29, 2025

I investigated this. The first source file passed to Csc is /mnt/vss/_work/1/s/.packages/polyfill/9.0.0-beta.14/contentFiles/cs/netstandard2.0/Polyfill/CallerArgumentExpressionAttribute.cs.

The analyzer implementation tries to get the editorconfig for the first source file here:

https://github.com/dotnet/roslyn/blob/6ad21c462cc84c2e22d12e7a1891547c9995c9a7/src/RoslynAnalyzers/PublicApiAnalyzers/Core/Analyzers/DeclarePublicApiAnalyzer.cs#L320-L327

However, polyfill is shipping an .editorconfig inside the package:

https://github.com/SimonCropp/Polyfill/blob/5168b2694414701125457937e704199f93a3aaa4/src/Polyfill/Polyfill.nuspec#L40-L41
https://github.com/SimonCropp/Polyfill/blob/5168b2694414701125457937e704199f93a3aaa4/src/Polyfill/exclude.editorconfig#L2

The editorconfig from polyfill is then considered and our dotnet_public_api_analyzer.require_api_files = true isn't considered.

dotnet_public_api_analyzer.require_api_files = true

Even if polyfill doesn't ship .editorconfig, I'm not sure how the "syntaxTrees.FirstOrDefault()" implementation on roslyn side will be able to find the right .editorconfig in this case.

@CyrusNajmabadi @333fred Can you please advise here? My feeling is that the correct way is to add require_api_files to .globalconfig (whose options are project-specific rather than syntax-tree/file specific) and Roslyn should then first respect .globalconfig, and fallback to .editorconfig for compatibility.

Evangelink
Evangelink previously approved these changes Jul 29, 2025
@Youssef1313
Copy link
Member Author

It looks like .globalconfig already work, at least locally. Let's see CI.

@Evangelink Evangelink enabled auto-merge (squash) July 30, 2025 07:22
@Evangelink Evangelink merged commit c361e3d into main Jul 30, 2025
8 checks passed
@Evangelink Evangelink deleted the dev/ygerges/analyzer-publicapi branch July 30, 2025 07:22
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.

3 participants