Skip to content

Conversation

@jjonescz
Copy link
Member

@jjonescz jjonescz commented Jul 9, 2025

Fixes #49670.

@jjonescz jjonescz added the Area-run-file Items related to the "dotnet run <file>" effort label Jul 9, 2025
@jjonescz jjonescz marked this pull request as ready for review July 9, 2025 12:01
@jjonescz jjonescz requested review from a team, RikkiGibson and Copilot July 9, 2025 12:01
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR ensures that the #:project directive fails when no argument is provided by updating the parsing logic and test coverage.

  • Adds "project" to the empty-name directive test to verify failure for project directives.
  • Extends Project.Parse signature to include directiveKind, checks for whitespace/empty names, and reports a missing-name error.
  • Updates the directive dispatch in CSharpDirective to call the new overload of Project.Parse.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
test/dotnet.Tests/CommandTests/Project/Convert/DotnetProjectConvertTests.cs Include "project" in Directives_EmptyName theory
src/Cli/dotnet/Commands/Run/VirtualProjectBuildingCommand.cs Fix call site to use new Project.Parse overload and add check for empty directive name
Comments suppressed due to low confidence (3)

test/dotnet.Tests/CommandTests/Project/Convert/DotnetProjectConvertTests.cs:813

  • Consider adding an assertion that inspects the thrown error's message to verify the MissingDirectiveName formatting for the project directive, ensuring the correct error text is emitted.
        [CombinatorialValues("sdk", "property", "package", "project")] string directive,

src/Cli/dotnet/Commands/Run/VirtualProjectBuildingCommand.cs:1216

  • [nitpick] The new modifier hides a potential Parse method in the base class (Named). Verify whether the base class defines a static Parse and remove new if hiding is unintentional.
        public static new Project? Parse(ImmutableArray<SimpleDiagnostic>.Builder? errors, SourceFile sourceFile, TextSpan span, string directiveKind, string directiveText)

src/Cli/dotnet/Commands/Run/VirtualProjectBuildingCommand.cs:1057

  • Double-check that all other invocations of Project.Parse have been updated to the new four-parameter overload to prevent missing-method compilation errors elsewhere.
            "project" => Project.Parse(errors, sourceFile, span, directiveKind, directiveText),

[Theory, CombinatorialData]
public void Directives_EmptyName(
[CombinatorialValues("sdk", "property", "package")] string directive,
[CombinatorialValues("sdk", "property", "package", "project")] string directive,
Copy link
Member

Choose a reason for hiding this comment

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

A virtual project will still be produced in this case right? I guess we also do that for the empty name cases for the other directives listed here.

Copy link
Member Author

Choose a reason for hiding this comment

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

In case of run-api, yes, that one collects all errors and produces the virtual project file (without directives that have errors), there are tests for that in general (Api_Diagnostic_*), albeit not this particular case. In case of other commands like run, the first error makes the whole command fail, so there is no virtual project produced.

@jjonescz jjonescz merged commit 301f9e9 into dotnet:main Jul 10, 2025
30 checks passed
@jjonescz jjonescz deleted the sprint-empty-project branch July 10, 2025 07:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area-run-file Items related to the "dotnet run <file>" effort

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Empty #:project appears to add a project item for the app directory

5 participants