Skip to content

Conversation

@arika0093
Copy link
Owner

@arika0093 arika0093 commented Nov 16, 2025

Refactor the project to use centralized source code generation through ConstSourceCodes, removing the DummyExpression class and unnecessary project references. This streamlines the codebase and reduces target frameworks in project files.

Summary by CodeRabbit

  • Breaking Changes

    • Removed DummyExpression class and related SelectExpr extension methods
    • Reduced supported target frameworks from netstandard2.0, net6.0, net8.0, net10.0 to netstandard2.0 on non-Windows platforms
  • Refactor

    • Streamlined source generator code export structure for improved maintainability

@coderabbitai
Copy link

coderabbitai bot commented Nov 16, 2025

Walkthrough

The PR refactors code generation in the Linqraft source generator by consolidating generated source templates into a dedicated ConstSourceCodes helper class, removes the compile-time DummyExpression.cs file (now generated at compile-time), updates target frameworks to netstandard2.0, and removes direct project references from build configurations.

Changes

Cohort / File(s) Summary
Source Generator Refactoring
src/Linqraft.SourceGenerator/ConstSourceCodes.cs, src/Linqraft.SourceGenerator/SelectExprGenerator.cs
New ConstSourceCodes class introduced to centralize source code templates (InterceptsLocationAttribute, DummyExpression, OverloadResolutionPriorityAttribute). SelectExprGenerator updated to delegate source export to ConstSourceCodes.ExportAll() instead of inline generation.
Removal of Compile-Time Dummy Implementation
src/Linqraft/DummyExpression.cs
Entire file removed; contained static SelectExpr extension methods for IQueryable<T> and IEnumerable<T>, OverloadResolutionPriorityAttribute, and related pragmas. Functionality now generated at compile-time.
Target Framework and Dependency Updates
src/Linqraft/Linqraft.csproj, src/Linqraft/packages.lock.json
Target frameworks narrowed from netstandard2.0;net6.0;net8.0;net10.0 to netstandard2.0 on non-Windows platforms. Lock file entries for net10.0, net6.0, and net8.0 removed, including their dependency graphs.
Build Configuration Updates
examples/Directory.Build.props, tests/Directory.Build.props, tests/Linqraft.Tests/Linqraft.Tests.csproj
Removed direct ProjectReference entries to Linqraft.csproj, eliminating compile-time linkage from examples and test projects to the main library.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Areas requiring extra attention:
    • Verify that ConstSourceCodes templates (InterceptsLocationAttribute, DummyExpression, OverloadResolutionPriorityAttribute) are correctly formed and cover all necessary generated source declarations previously in DummyExpression.cs
    • Confirm that SelectExprGenerator.cs correctly invokes ConstSourceCodes.ExportAll() and that the source generation pipeline produces identical output to the pre-refactoring implementation
    • Validate that removal of direct project references from test/example builds does not break compilation or test execution (ensure transitive dependencies are properly resolved)
    • Review target framework narrowing in Linqraft.csproj to confirm intentionality and assess impact on supported platforms

Possibly related PRs

  • feat: support .NET standard 2.0 #20: Modifies overlapping project metadata and target framework configurations (Linqraft.csproj, packages.lock.json, Directory.Build.props files), indicating a related refactoring or dependency management effort.

Poem

🐰 From dummy code to templates bright,
We move the generation to compile-time light—
DummyExpression hops away,
ConstSourceCodes leads the way!
Frameworks simplified, builds aligned,
A cleaner architecture we find! ✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately captures the main objectives of this changeset: centralizing source code generation via ConstSourceCodes and removing unnecessary project references.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/marker-code-move-generator

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between bf99460 and 1473f1d.

📒 Files selected for processing (8)
  • examples/Directory.Build.props (0 hunks)
  • src/Linqraft.SourceGenerator/ConstSourceCodes.cs (1 hunks)
  • src/Linqraft.SourceGenerator/SelectExprGenerator.cs (1 hunks)
  • src/Linqraft/DummyExpression.cs (0 hunks)
  • src/Linqraft/Linqraft.csproj (1 hunks)
  • src/Linqraft/packages.lock.json (0 hunks)
  • tests/Directory.Build.props (0 hunks)
  • tests/Linqraft.Tests/Linqraft.Tests.csproj (0 hunks)
💤 Files with no reviewable changes (5)
  • examples/Directory.Build.props
  • src/Linqraft/DummyExpression.cs
  • tests/Linqraft.Tests/Linqraft.Tests.csproj
  • tests/Directory.Build.props
  • src/Linqraft/packages.lock.json
🧰 Additional context used
🧬 Code graph analysis (1)
src/Linqraft.SourceGenerator/SelectExprGenerator.cs (1)
src/Linqraft.SourceGenerator/ConstSourceCodes.cs (2)
  • ConstSourceCodes (4-123)
  • ExportAll (7-11)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: build (windows-latest)
🔇 Additional comments (4)
src/Linqraft.SourceGenerator/SelectExprGenerator.cs (1)

21-22: LGTM! Clean refactor to centralized source generation.

The change to use ConstSourceCodes.ExportAll(ctx) centralizes the generation of predefined sources (InterceptsLocationAttribute and DummyExpression), improving maintainability and reducing code duplication.

src/Linqraft.SourceGenerator/ConstSourceCodes.cs (2)

7-11: LGTM! Clean implementation of centralized source export.

The ExportAll method correctly registers both generated sources with appropriate filenames.


14-33: LGTM! InterceptsLocationAttribute template is well-formed.

The attribute definition correctly follows the interceptor pattern with appropriate visibility (internal), AttributeUsage configuration, and property initialization.

src/Linqraft/Linqraft.csproj (1)

3-3: The review comment is based on an incorrect assumption about target framework compatibility.

Microsoft.CodeAnalysis.CSharp 4.13.0 targets and is compatible with .NETStandard 2.0, and the codebase contains no C# 6.0+ language features or APIs that require newer frameworks.

More importantly, removing specific target frameworks (net6.0, net8.0, net10.0) and targeting netstandard2.0 does not prevent consumers on those frameworks from using the library. Packages targeting netstandard2.0 are forward-compatible and can be consumed by any framework version equal to or newer than .NET Standard 2.0 (including net6.0, net8.0, net10.0). The test and example projects confirm this works correctly—they target net6.0+ and net10.0 while successfully consuming the netstandard2.0-targeted library.

Likely an incorrect or invalid review comment.

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.

2 participants