Do not report parse errors for '#:' in miscellaneous files #81385

RikkiGibson · 2025-11-21T01:42:28Z

Follow up to #81252

Removes unexpected errors for #: in miscellaneous files.
Fixes a completion crash for #:project $$ in the ctrl+N case. The file has a non-absolute path and we shouldn't try to use its containing directory as a base for path lookup.

Usually just the combination of: document is not part of an ordinary project, and document contains this directive, would make it be treated as an ordinary file. But there is also the case of virtual documents (not present on disk), containing these directives, e.g. when you hit ctrl+N in VS Code, set language mode to C# and start typing. Virtual documents are never treated as file-based programs, they are essentially only treated as miscellaneous files. But the user might be intending for this to be a file-based program, once they have saved it to disk. So reporting an error in this case is much more a nuisance than a benefit.

dibarbet

ide side lgtm

jcouv · 2025-11-21T07:09:43Z

Did we forget to create a test plan and label for file-based apps feature? #Closed

jcouv · 2025-11-21T07:28:52Z

src/Compilers/CSharp/Portable/CSharpParseOptions.cs

        /// In this mode, ignored directives <c>#:</c> are allowed.
        /// </remarks>
-        internal bool FileBasedProgram => Features.ContainsKey("FileBasedProgram");
+        internal bool AllowIgnoredDirectives => Features.ContainsKey("FileBasedProgram") || Features.ContainsKey("MiscellaneousFile");


Should those feature flags documented anywhere?
Not directly related to the PR: Aren't feature flags typically used for temporary or not fully supported scenarios? Should we consider exposing a proper API on parse options for file-based apps?

Perhaps, but, I will note that explicitly including these feature names in ordinary projects and similar to result in unspecified behavior. These are generally supposed to be only included by the tooling.

It’s possible there is a more preferred way of “tagging” compilations in the desired way here than just using special feature names. Happy to have that discussion in more depth.

jcouv · 2025-11-21T07:35:03Z

src/Compilers/CSharp/Portable/Parser/DirectiveParser.cs

            if (isActive)
            {
-                if (!lexer.Options.FileBasedProgram)
+                if (!lexer.Options.AllowIgnoredDirectives)


Sorry for my lack of context, but I'm surprised that the parsing of ignored directives is conditional in the first place. The spec only says we're adding directives to the language and that they are ignored: https://github.com/dotnet/csharplang/blob/main/proposals/csharp-14.0/ignored-directives.md #Closed

AFAIK, this only controls whether an error is reported on usage.

Not blocking this PR: should we update the spec or remove the condition? In other words, did we intentionally fork the language?

The spec says:

Compilers are also free to report errors if these directives are used in unsupported scenarios

It's possible we want to push some of those rules into the language, but it's not clear to me. I think the mechanism that decides whether the scenario is supported would also have to be defined in the language spec.

Yes, reporting an error on #: directives in normal project-based apps is intentional. That is because those directives are ignored for project-based apps and users might be confused if they were ignored silently. This is only a tooling feature though, i.e., roslyn implements it; sdk tells it the kind of the program (file-based vs project-based) via passing the feature flag. It is not a language feature since language has no way to interpret those directives.

That is because those directives are ignored for project-based apps and users might be confused if they were ignored silently.

That behavior makes sense.
Still, the net result is that we have forked the language, but I don't recall that was discussed as part of the language design. The spec only says we add these to the grammar and they are ignored, but our implementation diverges (errors in some cases). We're using a feature flag for a long-living feature, which smells.

I'll start an email thread.

jcouv · 2025-11-21T07:49:41Z

...torFeatures/CSharpTest/Completion/CompletionProviders/AppDirectiveCompletionProviderTests.cs

+
+        var markup = $"""
+            <Workspace>
+                <Project Language="C#" CommonReferences="true" AssemblyName="Test1" Features="FileBasedProgram=true">


Features="FileBasedProgram=true"

Did this test fail before the changes in this PR?
The reason I'm asking is because I see logic elsewhere that adds the "MiscellaneousFile" feature and therefore it's not clear whether this test should have the "FileBasedProgram" feature flag. #Closed

No, it passed. It’s reasonable for some of these tests to use FileBasedProgram feature flag, because we want to see how the editor features behave on file-based programs.

However I do think the actual applications of FileBasedProgram versus MiscellaneousFile feature names requires some discussion.

From the description of the bug being fixed, I expected a test like this one but without FileBasedProgram and with MiscellaneousFile. It used to fail, but now it would pass. Did I understand right?

Resolving by deleting MiscellaneousFile flag from the compiler layer. (i.e. reverting all compiler changes in this PR.)

...torFeatures/CSharpTest/Completion/CompletionProviders/AppDirectiveCompletionProviderTests.cs

jcouv

Done with review pass (commit 1) for compiler. Only glanced at IDE code

jjonescz · 2025-11-24T10:18:16Z

Did we forget to create a test plan and label for file-based apps feature?

We have a label: Feature - Run File #: and #! directives and file-based C# programs
I don't think we have a test plan; IIRC that might have been intentional since the relevant language feature "ignored directives" was merged in a single PR. #Closed

…file-documents

…ogram flag to miscellaneous document.

RikkiGibson · 2025-12-10T21:27:08Z

src/Features/Core/Portable/Workspace/MiscellaneousFileUtilities.cs

+            else
+            {
+                // Any non-script misc file should not complain about usage of '#:' ignored directives.
+                parseOptions = parseOptions.WithFeatures([.. parseOptions.Features, new("FileBasedProgram", "true")]);


This change makes it so if there are parse options, then we are either going to treat this as a script, or we'll pass the FileBasedProgram flag (to avoid reporting possibly unwanted errors for #:). Not neither, not both.

RikkiGibson · 2025-12-10T21:28:06Z

...ageServer/Microsoft.CodeAnalysis.LanguageServer.UnitTests/FileBasedProgramsWorkspaceTests.cs

+        // FileBasedProgram feature flag is not passed, so an error is expected on '#:'.
+        var primordialSyntaxTree = await primordialDocument.GetRequiredSyntaxTreeAsync(CancellationToken.None);
+        primordialSyntaxTree.GetDiagnostics(CancellationToken.None).Verify(
+            // script.csx(1,2): error CS9298: '#:' directives can be only used in file-based programs ('-features:FileBasedProgram')"


We wouldn't want to report this error for a misc file like Untitled-1, which is likely to be ordinary .cs code. But I think it's good to report it when we know it is a .csx script.

Do not report parse errors for '#:' in miscellaneous files

ffc8135

RikkiGibson requested review from a team as code owners November 21, 2025 01:42

github-actions bot added the Area-Compilers label Nov 21, 2025

dotnet-policy-service bot added the VSCode label Nov 21, 2025

dibarbet approved these changes Nov 21, 2025

View reviewed changes

jcouv reviewed Nov 21, 2025

View reviewed changes

...torFeatures/CSharpTest/Completion/CompletionProviders/AppDirectiveCompletionProviderTests.cs Outdated Show resolved Hide resolved

jcouv reviewed Nov 21, 2025

View reviewed changes

...torFeatures/CSharpTest/Completion/CompletionProviders/AppDirectiveCompletionProviderTests.cs Outdated Show resolved Hide resolved

jcouv reviewed Nov 21, 2025

View reviewed changes

jcouv self-assigned this Nov 21, 2025

jjonescz added the Feature - Run File #: and #! directives and file-based C# programs label Nov 21, 2025

Merge remote-tracking branch 'upstream/main' into fbp-directives-non-…

14b074e

…file-documents

RikkiGibson mentioned this pull request Dec 10, 2025

csx scripts should not be treated as file-based programs #81644

Open

Back out Compilers change. Adjust conditions for applying FileBasedPr…

e277588

…ogram flag to miscellaneous document.

RikkiGibson commented Dec 10, 2025

View reviewed changes

RikkiGibson requested a review from dibarbet December 10, 2025 21:28

RikkiGibson added Area-IDE and removed Area-Compilers labels Dec 10, 2025

dibarbet approved these changes Dec 10, 2025

View reviewed changes

RikkiGibson enabled auto-merge (squash) December 10, 2025 22:28

RikkiGibson merged commit 46a48b8 into dotnet:main Dec 10, 2025
25 of 26 checks passed

dotnet-policy-service bot added this to the Next milestone Dec 10, 2025

RikkiGibson deleted the fbp-directives-non-file-documents branch December 10, 2025 23:24

dotnet-maestro bot mentioned this pull request Dec 11, 2025

[release/10.0.2xx] Source code updates from dotnet/roslyn dotnet/dotnet#3816

Merged

dotnet-bot mentioned this pull request Dec 12, 2025

[Automated] PRs inserted in VS build main-11311.198 #81672

Closed

Do not report parse errors for '#:' in miscellaneous files #81385

Do not report parse errors for '#:' in miscellaneous files #81385

Conversation

RikkiGibson commented Nov 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dibarbet left a comment

Choose a reason for hiding this comment

Uh oh!

jcouv commented Nov 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jcouv Nov 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jcouv Nov 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RikkiGibson Dec 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

jcouv left a comment

Choose a reason for hiding this comment

Uh oh!

jjonescz commented Nov 24, 2025 • edited by jcouv Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RikkiGibson Dec 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

RikkiGibson commented Nov 21, 2025 •

edited

Loading

jcouv commented Nov 21, 2025 •

edited

Loading

jcouv Nov 21, 2025 •

edited

Loading

jcouv Nov 21, 2025 •

edited

Loading

RikkiGibson Dec 10, 2025 •

edited

Loading

jjonescz commented Nov 24, 2025 •

edited by jcouv

Loading

RikkiGibson Dec 10, 2025 •

edited

Loading