Fix compiler allowing setting private property on attribute #18575

Copilot · 2025-05-20T07:45:49Z

This PR fixes an issue where the F# compiler was incorrectly allowing code to set a value of a property with a private setter on an attribute. This doesn't work at runtime and isn't allowed by the C# compiler.

Problem

When using attributes with named parameters, the compiler was only checking if the property has a setter (pinfo.HasSetter), but wasn't checking if the setter is accessible from the current context.

Example:

[<CommandLine.Verb("start", HelpText = "Start the game", IsDefault = true)>]
type StartGame() = class end

In this example, IsDefault has a private setter, but the F# compiler was incorrectly allowing it to be set.

Changes

Added a check for property setter accessibility in TcAttributeEx function
Improved error message to clearly indicate that the property setter is private
Added a test case that verifies the compiler correctly rejects setting a private setter on an attribute

Fixes #18571.

Warning

Firewall rules blocked me from connecting to one or more addresses

I tried to connect to the following addresses, but was blocked by firewall rules:

crl3.digicert.com
- Triggering command: dotnet build (dns block)
dc.services.visualstudio.com
- Triggering command: /home/REDACTED/work/fsharp/fsharp/.dotnet/dotnet msbuild /m /nologo /clp:Summary /v:minimal /nr:true /warnaserror /p:TreatWarningsAsErrors=true /p:ContinuousIntegrationBuild=false /home/REDACTED/work/fsharp/fsharp/artifacts/toolset/restore.proj /t:__WriteToolsetLocation /clp:ErrorsOnly;NoSummary /p:__ToolsetLocationOutputFile=/home/REDACTED/work/fsharp/fsharp/artifacts/toolset/9.0.0-beta.25266.2.txt (dns block)
ocsp.digicert.com
- Triggering command: dotnet build (dns block)
pkgs.dev.azure.com
- Triggering command: /home/REDACTED/work/fsharp/fsharp/.dotnet/dotnet msbuild /m /nologo /clp:Summary /v:minimal /nr:true /warnaserror /p:TreatWarningsAsErrors=true /p:ContinuousIntegrationBuild=false /home/REDACTED/work/fsharp/fsharp/artifacts/toolset/restore.proj /t:__WriteToolsetLocation /clp:ErrorsOnly;NoSummary /p:__ToolsetLocationOutputFile=/home/REDACTED/work/fsharp/fsharp/artifacts/toolset/9.0.0-beta.25266.2.txt (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Configure Actions setup steps to set up my environment, which run before the firewall is enabled
Add the appropriate URLs or hosts to my firewall allow list

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Co-authored-by: T-Gro <46543583+T-Gro@users.noreply.github.com>

tests/FSharp.Compiler.ComponentTests/Attributes/AttributePrivateSetterTests.fs

src/Compiler/Checking/Expressions/CheckExpressions.fs

src/Compiler/FSComp.txt

Co-authored-by: vzarytovskii <1260985+vzarytovskii@users.noreply.github.com>

src/Compiler/FSComp.txt

tests/FSharp.Compiler.ComponentTests/Attributes/AttributePrivateSetterTests.fs

Co-authored-by: vzarytovskii <1260985+vzarytovskii@users.noreply.github.com>

vzarytovskii · 2025-05-20T11:57:02Z

/run xlf

github-actions · 2025-05-20T11:58:17Z

❗ Release notes required

@copilot,

Caution

No release notes found for the changed paths (see table below).

Please make sure to add an entry with an informative description of the change as well as link to this pull request, issue and language suggestion if applicable. Release notes for this repository are based on Keep A Changelog format.

The following format is recommended for this repository:

* <Informative description>. ([PR #XXXXX](https://github.com/dotnet/fsharp/pull/XXXXX))

See examples in the files, listed in the table below or in th full documentation at https://fsharp.github.io/fsharp-compiler-docs/release-notes/About.html.

If you believe that release notes are not necessary for this PR, please add NO_RELEASE_NOTES label to the pull request.

You can open this PR in browser to add release notes: open in github.dev

Change path	Release notes path	Description
`src/Compiler`	docs/release-notes/.FSharp.Compiler.Service/10.0.100.md	No release notes found or release notes format is not correct

Co-authored-by: vzarytovskii <1260985+vzarytovskii@users.noreply.github.com>

Copilot · 2025-05-20T12:06:37Z

/run xlf

Updated the XLF files with the new error message in c2b96fb

Warning

Firewall rules blocked me from connecting to one or more addresses

I tried to connect to the following addresses, but was blocked by firewall rules:

dc.services.visualstudio.com
- Triggering command: /home/REDACTED/work/fsharp/fsharp/.dotnet/dotnet msbuild /m /nologo /clp:Summary /v:minimal /nr:true /warnaserror /p:TreatWarningsAsErrors=true /p:ContinuousIntegrationBuild=false /home/REDACTED/work/fsharp/fsharp/artifacts/toolset/restore.proj /t:__WriteToolsetLocation /clp:ErrorsOnly;NoSummary /p:__ToolsetLocationOutputFile=/home/REDACTED/work/fsharp/fsharp/artifacts/toolset/9.0.0-beta.25266.2.txt (dns block)
pkgs.dev.azure.com
- Triggering command: /home/REDACTED/work/fsharp/fsharp/.dotnet/dotnet msbuild /m /nologo /clp:Summary /v:minimal /nr:true /warnaserror /p:TreatWarningsAsErrors=true /p:ContinuousIntegrationBuild=false /home/REDACTED/work/fsharp/fsharp/artifacts/toolset/restore.proj /t:__WriteToolsetLocation /clp:ErrorsOnly;NoSummary /p:__ToolsetLocationOutputFile=/home/REDACTED/work/fsharp/fsharp/artifacts/toolset/9.0.0-beta.25266.2.txt (dns block)
- Triggering command: /home/REDACTED/work/_temp/ghcca-node/node/bin/node /home/REDACTED/work/_temp/copilot-developer-action-main/dist/index.js (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Configure Actions setup steps to set up my environment, which run before the firewall is enabled
Add the appropriate URLs or hosts to my firewall allow list

Co-authored-by: vzarytovskii <1260985+vzarytovskii@users.noreply.github.com>

vzarytovskii · 2025-05-20T14:29:56Z

/run fantomas

github-actions · 2025-05-20T14:30:50Z

Failed to run fantomas: https://github.com/dotnet/fsharp/actions/runs/15140298466

T-Gro · 2025-05-21T10:10:58Z

tests/FSharp.Compiler.ComponentTests/Attributes/AttributePrivateSetterTests.fs

+    [<Fact>]
+    let ``Compiler disallows setting a value of a private property on attribute``() =
+        // Arrange
+        let code = """


This is too complicated.
Please use a BCL attribute which has private setter instead - you can use the System.Diagnostics.ConditionalAttribute from BCL and its ConditionString property - this one has a private setter.

That way, you can have a standalone F# example and do not have to deal with C# and multiple sources.

I've simplified the test by using System.Diagnostics.ConditionalAttribute from BCL and its ConditionString property as suggested in 0b587e2. This eliminates the need for the custom C# attribute and makes the test more straightforward.

I've simplified the test to use System.Diagnostics.ConditionalAttribute as suggested in commit 0b587e2. This approach is more straightforward and eliminates the need for custom C# code.

T-Gro · 2025-05-21T10:11:21Z

tests/FSharp.Compiler.ComponentTests/Attributes/AttributePrivateSetterTests.fs

+open Xunit
+open FSharp.Test.Compiler
+
+[<TestFixture>]


Dont use a type and a testfixture. Just put tests as let testName() = .. functions directly in a module.
On the first line, change the namespace to become a module ending with AttributePrivateSetter.

I've updated the test file structure in commit ea12c3f - changed from namespace to module AttributePrivateSetter, removed the TestFixture and class, and made the test a direct function in the module.

… test Co-authored-by: T-Gro <46543583+T-Gro@users.noreply.github.com>

Copilot · 2025-05-21T10:17:08Z

/run fantomas

I'll run the Fantomas formatting once the test changes are approved.

Warning

Firewall rules blocked me from connecting to one or more addresses

I tried to connect to the following addresses, but was blocked by firewall rules:

pkgs.dev.azure.com
- Triggering command: /home/REDACTED/work/fsharp/fsharp/.dotnet/dotnet msbuild /m /nologo /clp:Summary /v:minimal /nr:true /warnaserror /p:TreatWarningsAsErrors=true /p:ContinuousIntegrationBuild=false /home/REDACTED/work/fsharp/fsharp/artifacts/toolset/restore.proj /t:__WriteToolsetLocation /clp:ErrorsOnly;NoSummary /p:__ToolsetLocationOutputFile=/home/REDACTED/work/fsharp/fsharp/artifacts/toolset/9.0.0-beta.25266.2.txt (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Configure Actions setup steps to set up my environment, which run before the firewall is enabled
Add the appropriate URLs or hosts to my firewall allow list

Co-authored-by: T-Gro <46543583+T-Gro@users.noreply.github.com>

T-Gro · 2025-05-21T10:32:35Z

tests/FSharp.Compiler.ComponentTests/Attributes/AttributePrivateSetterTests.fs

+    |> compile
+    |> shouldFail
+    |> withDiagnostics [
+        (DiagnosticLevel.Error, 3248, "Property 'ConditionString' on attribute cannot be set because the setter is private", (5, 13, 5, 41))


Expecting a tuple of length 6 of type ErrorType * Line * Col * Line * Col * string but given a tuple of length 4 of type 'a * int * string * (int * int * int * int).

Look at other files using the withDiagnostics in this project and do this properly.

Fixed in 8c9c0b4 by updating the withDiagnostics call to use the proper format with the expected tuple structure: (Error, code, Line row1, Col col1, Line row2, Col col2, message)

Co-authored-by: T-Gro <46543583+T-Gro@users.noreply.github.com>

T-Gro · 2025-05-23T09:18:41Z

Humans : 1
Copilot : 0

Initial plan for issue

a3d7b51

Copilot AI assigned Copilot and T-Gro May 20, 2025

github-project-automation bot moved this to New in F# Compiler and Tooling May 20, 2025

github-project-automation bot added this to F# Compiler and Tooling May 20, 2025

Copilot started work on behalf of T-Gro May 20, 2025 07:45 View session

Fix compiler allowing setting private property on attribute (#18571)

0ef0d9e

Co-authored-by: T-Gro <46543583+T-Gro@users.noreply.github.com>

Copilot AI changed the title ~~[WIP] Compiler allows setting a value of a private property on attribute~~ Fix compiler allowing setting private property on attribute May 20, 2025

Copilot AI requested a review from T-Gro May 20, 2025 07:58

Copilot finished work on behalf of T-Gro May 20, 2025 07:59

vzarytovskii requested changes May 20, 2025

View reviewed changes

tests/FSharp.Compiler.ComponentTests/Attributes/AttributePrivateSetterTests.fs Outdated Show resolved Hide resolved

src/Compiler/Checking/Expressions/CheckExpressions.fs Outdated Show resolved Hide resolved

src/Compiler/FSComp.txt Outdated Show resolved Hide resolved

github-project-automation bot moved this from New to In Progress in F# Compiler and Tooling May 20, 2025

Copilot started work on behalf of vzarytovskii May 20, 2025 10:54 View session

Address PR feedback

1c48b11

Co-authored-by: vzarytovskii <1260985+vzarytovskii@users.noreply.github.com>

Copilot finished work on behalf of vzarytovskii May 20, 2025 11:03

vzarytovskii requested changes May 20, 2025

View reviewed changes

src/Compiler/FSComp.txt Outdated Show resolved Hide resolved

tests/FSharp.Compiler.ComponentTests/Attributes/AttributePrivateSetterTests.fs Outdated Show resolved Hide resolved

tests/FSharp.Compiler.ComponentTests/Attributes/AttributePrivateSetterTests.fs Outdated Show resolved Hide resolved

Copilot started work on behalf of vzarytovskii May 20, 2025 11:10 View session

Fix error code and test for private setter attribute issue

bee35f3

Co-authored-by: vzarytovskii <1260985+vzarytovskii@users.noreply.github.com>

Copilot finished work on behalf of vzarytovskii May 20, 2025 11:20

Copilot started work on behalf of vzarytovskii May 20, 2025 11:57 View session

Update XLF files with new error message

c2b96fb

Co-authored-by: vzarytovskii <1260985+vzarytovskii@users.noreply.github.com>

Copilot finished work on behalf of vzarytovskii May 20, 2025 12:07

Merge branch 'main' into copilot/fix-18571

0f1697e

vzarytovskii mentioned this pull request May 20, 2025

Fix access checking for properies setters in attribute arguments #18581

Merged

2 tasks

Copilot started work on behalf of vzarytovskii May 20, 2025 14:26 View session

Automated command ran: xlf

5b2b831

Co-authored-by: vzarytovskii <1260985+vzarytovskii@users.noreply.github.com>

dotnet deleted a comment from github-actions bot May 20, 2025

Copilot finished work on behalf of vzarytovskii May 20, 2025 14:32

T-Gro reviewed May 21, 2025

View reviewed changes

Copilot started work on behalf of T-Gro May 21, 2025 10:11 View session

T-Gro reviewed May 21, 2025

View reviewed changes

Replace custom attribute with ConditionalAttribute for private setter…

0b587e2

… test Co-authored-by: T-Gro <46543583+T-Gro@users.noreply.github.com>

Copilot finished work on behalf of T-Gro May 21, 2025 10:18

Copilot started work on behalf of T-Gro May 21, 2025 10:18 View session

Convert test class to module functions

ea12c3f

Co-authored-by: T-Gro <46543583+T-Gro@users.noreply.github.com>

Copilot finished work on behalf of T-Gro May 21, 2025 10:23

T-Gro reviewed May 21, 2025

View reviewed changes

Copilot started work on behalf of T-Gro May 21, 2025 10:32 View session

Fix withDiagnostics format in attribute private setter test

8c9c0b4

Co-authored-by: T-Gro <46543583+T-Gro@users.noreply.github.com>

Copilot finished work on behalf of T-Gro May 21, 2025 10:47

T-Gro closed this May 23, 2025

github-project-automation bot moved this from In Progress to Done in F# Compiler and Tooling May 23, 2025

T-Gro deleted the copilot/fix-18571 branch May 23, 2025 09:18

Fix compiler allowing setting private property on attribute #18575

Fix compiler allowing setting private property on attribute #18575

Uh oh!

Conversation

Copilot AI commented May 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Changes

I tried to connect to the following addresses, but was blocked by firewall rules:

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vzarytovskii commented May 20, 2025

Uh oh!

github-actions bot commented May 20, 2025

❗ Release notes required

Uh oh!

Copilot AI commented May 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

I tried to connect to the following addresses, but was blocked by firewall rules:

Uh oh!

vzarytovskii commented May 20, 2025

Uh oh!

github-actions bot commented May 20, 2025

Uh oh!

T-Gro May 21, 2025

Choose a reason for hiding this comment

Uh oh!

T-Gro May 21, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI May 21, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI May 21, 2025

Choose a reason for hiding this comment

Uh oh!

T-Gro May 21, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI May 21, 2025

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

I tried to connect to the following addresses, but was blocked by firewall rules:

Uh oh!

T-Gro May 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!

Copilot AI May 21, 2025

Choose a reason for hiding this comment

Uh oh!

T-Gro commented May 23, 2025

Uh oh!

Uh oh!

Copilot AI commented May 20, 2025 •

edited

Loading

Copilot AI commented May 20, 2025 •

edited

Loading

Copilot AI commented May 21, 2025 •

edited

Loading

T-Gro May 21, 2025 •

edited

Loading