feat: code scanning cleanup and Roslyn analyzers#244
Conversation
Summary of ChangesHello @joshsmithxrm, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the project's code quality and developer experience by streamlining static analysis tools and introducing custom architectural enforcement. By tuning existing code scanning tools and integrating new Roslyn analyzers, the aim is to reduce irrelevant warnings, highlight critical issues more effectively, and ensure adherence to established architectural patterns, ultimately leading to more robust and maintainable code. Highlights
Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This is an excellent pull request that significantly improves the project's code quality and maintainability by introducing Roslyn analyzers for architectural enforcement and cleaning up code scanning configurations. The new documentation, analyzer projects, and configuration files are well-structured and clear. I have one suggestion to make one of the new analyzers more robust by using a stronger type check. Overall, this is a high-quality contribution.
There was a problem hiding this comment.
Pull request overview
This PR implements a comprehensive code scanning cleanup by tuning existing tools (CodeQL, Copilot, Gemini) and introducing custom Roslyn analyzers for compile-time architectural enforcement. The changes focus on reducing noise from style suggestions while maintaining security analysis capabilities.
- Configures CodeQL to disable 6 style rules that conflict with project conventions while keeping the security-and-quality suite
- Adds custom Roslyn analyzers (PPDS.Analyzers) for architectural pattern enforcement at compile-time
- Creates bot configuration files to guide Copilot and Gemini reviews toward real issues (concurrency, performance, resource leaks)
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
src/PPDS.Analyzers/Rules/UseEarlyBoundEntitiesAnalyzer.cs |
Analyzer to detect string literals in QueryExpression and suggest early-bound constants |
src/PPDS.Analyzers/Rules/NoSyncOverAsyncAnalyzer.cs |
Analyzer to detect sync-over-async patterns that can cause deadlocks |
src/PPDS.Analyzers/Rules/NoFireAndForgetInCtorAnalyzer.cs |
Analyzer to detect unawaited async calls in constructors |
src/PPDS.Analyzers/DiagnosticIds.cs |
Central registry of diagnostic IDs and categories for all analyzers |
src/PPDS.Analyzers/PPDS.Analyzers.csproj |
Analyzer project configuration targeting netstandard2.0 with Roslyn packages |
src/PPDS.Cli/PPDS.Cli.csproj |
Adds analyzer project reference with OutputItemType="Analyzer" |
src/PPDS.Cli/Services/ServiceRegistration.cs |
Adds pragma suppression for PPDS012 with justification comment for DI factory |
PPDS.Sdk.sln |
Adds PPDS.Analyzers project to solution with build configurations |
.github/copilot-instructions.md |
Guidance for Copilot reviews based on ADRs and project patterns |
.github/codeql/codeql-config.yml |
Disables 6 style-related CodeQL rules while keeping security analysis |
.github/CODE_SCANNING.md |
Comprehensive documentation of code scanning tools and rationale |
.gemini/config.yaml |
Gemini Code Assist configuration with MEDIUM severity threshold |
.gemini/styleguide.md |
Architecture guidance for Gemini based on ADRs and common patterns |
.editorconfig |
Updates generated code pattern and adds CS8981 suppression for lowercase enums |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Tune code scanning tools to reduce noise and add architectural enforcement: ## CodeQL - Disable 6 style rules that conflict with project conventions - Keep security-and-quality suite for actual security analysis - All 282 existing alerts dismissed (batch API) ## Bot Configuration - Add .github/copilot-instructions.md with ADR-based guidance - Add .gemini/config.yaml and styleguide.md for Gemini Code Assist - Focus bots on real issues: concurrency, performance, resource leaks ## Roslyn Analyzers (PPDS.Analyzers) - PPDS006: UseEarlyBoundEntities - flag string literals in QueryExpression - PPDS012: NoSyncOverAsync - flag .GetAwaiter().GetResult(), .Result, .Wait() - PPDS013: NoFireAndForgetInCtor - flag unawaited async calls in constructors ## EditorConfig - Fix generated code pattern to match subdirectories - Suppress CS8981 for lowercase enum names in generated code Closes #231 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
feefac2 to
8af8697
Compare
- Use full metadata name for QueryExpression type check (Gemini) - Reference CODE_SCANNING.md instead of CLAUDE.md (Copilot) - Combine nested if statements in NoSyncOverAsyncAnalyzer (Copilot) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add PPDS012/PPDS013 suppressions to TUI files with comments explaining why: - PpdsApplication.cs: Terminal.Gui requires sync disposal (IDisposable contract) - SqlQueryScreen.cs: Fire-and-forget with explicit ContinueWith error handling 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add triage decision tree to CODE_SCANNING.md - Document known safe patterns (Terminal.Gui sync disposal, DI factories, ContinueWith) - Add HasContinueWithErrorHandling to PPDS013 analyzer to recognize fire-and-forget with .ContinueWith() as intentional error handling - Reduces false positives for patterns like SqlQueryScreen constructor Closes #246 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
CodeQL flagged outerInvocation as unused - only type check needed. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
Fixed CodeQL finding (unused variable) in 8d73174 - Removed |
Summary
Changes
CodeQL Configuration
Bot Configuration
.github/copilot-instructions.mdwith ADR-based architecture guidance.gemini/config.yamlandstyleguide.mdfor Gemini Code AssistRoslyn Analyzers (PPDS.Analyzers)
.GetAwaiter().GetResult(),.Result,.Wait()EditorConfig
**/Generated/**.cs)Test plan
Closes #231
🤖 Generated with Claude Code