Skip to content

Allow access modifiers to auto properties getters and setters - #16861

Merged
vzarytovskii merged 65 commits into
dotnet:mainfrom
ijklam:access-modifies-to-auto-properties
Aug 12, 2024
Merged

Allow access modifiers to auto properties getters and setters#16861
vzarytovskii merged 65 commits into
dotnet:mainfrom
ijklam:access-modifies-to-auto-properties

Conversation

@ijklam

@ijklamijklam commented Mar 12, 2024

Copy link
Copy Markdown
Contributor

Description

Fixes#16854

Checklist

  • Test cases added
  • sig file support
  • new syntax in fsi printing
    图片
  • new syntax in quickinfo
    图片
  • new syntax in sig file generation
moduleProgramtypeA()=member valinternalB:int =0with get, set
member valD:int =0withinternal get,private set
memberinternal_.Fwith get()=1andset(v:int)=()memberinternal_.Gwith get(x:int)=1andset(x:int)(v:int)=()// above generatesmoduleProgramtypeA=new: unit -> A
memberB:int with internal get,internal setmemberD:int with internal get,private setmemberF:int with internal get,internal setmemberG:x:int ->int with internal get,internal set
  • Performance benchmarks added in case of performance changes
  • Release notes entry updated:

@github-actions

github-actionsBot commented Mar 12, 2024

Copy link
Copy Markdown
Contributor

❗ Release notes required


✅ Found changes and release notes in following paths:

Change pathRelease notes pathDescription
src/Compilerdocs/release-notes/.FSharp.Compiler.Service/9.0.100.md
LanguageFeatures.fsidocs/release-notes/.Language/preview.md

@ijklamijklam changed the title Access modifies to auto propertiesAllow access modifiers to auto properties getters and settersMar 12, 2024
support for tooling
@vzarytovskii

Copy link
Copy Markdown
Member

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 2 pipeline(s).

@edgarfgp

edgarfgp commented Aug 7, 2024

Copy link
Copy Markdown
Contributor

This seems to be ready, apart from conflicts. Would be good to have this in please 🙏

@psfinaki

Copy link
Copy Markdown
Contributor

@Tangent-90 will you have time to get to this or maybe you have fresh thoughts? If no - which is understood since we didn't get to this in a timely manner - I can probably fix the conflicts and give it a proper review next week.

@ijklam

Copy link
Copy Markdown
ContributorAuthor

@Tangent-90 will you have time to get to this or maybe you have fresh thoughts? If no - which is understood since we didn't get to this in a timely manner - I can probably fix the conflicts and give it a proper review next week.

Sorry for the lately response, I may try to fix it in this weekend.

3869,featureParsedHashDirectiveUnexpectedInteger,"Unexpected integer literal '%d'."
3869,featureParsedHashDirectiveUnexpectedIdentifier,"Unexpected identifier '%s'."
3870,featureParsedHashDirectiveUnexpectedInteger,"Unexpected integer literal '%d'."
3870,featureParsedHashDirectiveUnexpectedIdentifier,"Unexpected identifier '%s'."

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Mhm this is probably okay but to be on the safe side I would not change warning numbers here. I know this happened because another features got introduced since this PR was created, sorry for inconvenience therefore.

Comment threadsrc/Compiler/pars.fsy

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@auduchinok if you're nearby, PTAL if you're okay with the parser changes (look reasonable to me).

/// Gets the syntax range of this construct
member Range: range

/// Represents two access

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
/// Represents two access
/// Represents two access modifiers

Comment threadsrc/Compiler/Checking/infos.fs

@psfinakipsfinaki left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Good stuff, thanks for adding the signature support and extra tests. Left a few small remarks.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

Redo #16687 (Allow access modifiers to auto properties getters and setters)

7 participants

@ijklam@nojaf@abonie@abelbraaksma@edgarfgp@vzarytovskii@psfinaki