Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conventions/Naming: rename Accessibility DU to AccessControlLevel #565

Merged
merged 1 commit into from
Oct 16, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ let private getValueOrFunctionIdents _ pattern =
let private getIdentifiers (args:AstNodeRuleParams) =
match args.AstNode with
| AstNode.Expression(SynExpr.ForEach(_, _, true, pattern, _, _, _)) ->
getPatternIdents Accessibility.Private getValueOrFunctionIdents false pattern
getPatternIdents AccessControlLevel.Private getValueOrFunctionIdents false pattern
| AstNode.Binding(SynBinding(_, _, _, _, attributes, _, valData, pattern, _, _, _, _)) ->
if not (isLiteral attributes) then
match identifierTypeFromValData valData with
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ let private getValueOrFunctionIdents typeChecker isPublic pattern =
match List.tryLast longIdent.Lid with
| Some ident when not (isActivePattern ident) && singleIdentifier ->
let checkNotUnionCase = checkNotUnionCase ident
if isPublic = Accessibility.Internal then
if isPublic = AccessControlLevel.Internal then
(ident, ident.idText, Some checkNotUnionCase)
|> Array.singleton
else
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ let private getIdentifiers (args:AstNodeRuleParams) =
if not (isLiteral attributes) && not (isImplementingInterface parents) then
match identifierTypeFromValData valData with
| Member | Property ->
getPatternIdents Accessibility.Private getMemberIdents true pattern
getPatternIdents AccessControlLevel.Private getMemberIdents true pattern
| _ -> Array.empty
else
Array.empty
Expand Down
33 changes: 19 additions & 14 deletions src/FSharpLint.Core/Rules/Conventions/Naming/NamingHelper.fs
Original file line number Diff line number Diff line change
Expand Up @@ -179,16 +179,21 @@ let activePatternIdentifiers (identifier:Ident) =
|> Array.filter (fun x -> not <| String.IsNullOrEmpty(x) && x.Trim() <> "_")


type Accessibility =
/// Specifies access control level as described in
/// https://docs.microsoft.com/en-us/dotnet/fsharp/language-reference/access-control .
/// Higher levels also include lower levels, so e.g. identifier marked with Public
/// is also accessible in Internal and Private scopes.
/// Public scope is the widest, then goes Internal, then Private.
type AccessControlLevel =
| Public
| Private
| Internal

let getAccessibility (syntaxArray:AbstractSyntaxArray.Node []) i =
let isSynAccessPublic = function
| Some(SynAccess.Public) | None -> Accessibility.Public
| Some(SynAccess.Private) -> Accessibility.Private
| Some(SynAccess.Internal) -> Accessibility.Internal
| Some(SynAccess.Public) | None -> AccessControlLevel.Public
| Some(SynAccess.Private) -> AccessControlLevel.Private
| Some(SynAccess.Internal) -> AccessControlLevel.Internal

let rec getAccessibility state isPrivateWhenReachedBinding i =
if i = 0 then state
Expand All @@ -206,12 +211,12 @@ let getAccessibility (syntaxArray:AbstractSyntaxArray.Node []) i =
| Pattern(SynPat.LongIdent(_, _, _, _, access, _)) ->
getAccessibility (isSynAccessPublic access) isPrivateWhenReachedBinding node.ParentIndex
| TypeSimpleRepresentation(_)
| Pattern(_) -> Accessibility.Public
| Pattern(_) -> AccessControlLevel.Public
| MemberDefinition(_) ->
if isPrivateWhenReachedBinding then Accessibility.Private
if isPrivateWhenReachedBinding then AccessControlLevel.Private
else getAccessibility state isPrivateWhenReachedBinding node.ParentIndex
| Binding(SynBinding(access, _, _, _, _, _, _, _, _, _, _, _)) ->
if isPrivateWhenReachedBinding then Accessibility.Private
if isPrivateWhenReachedBinding then AccessControlLevel.Private
else getAccessibility (isSynAccessPublic access) true node.ParentIndex
| EnumCase(_)
| TypeRepresentation(_)
Expand All @@ -231,7 +236,7 @@ let getAccessibility (syntaxArray:AbstractSyntaxArray.Node []) i =
| LambdaBody(_)
| Expression(_) -> getAccessibility state true node.ParentIndex

getAccessibility Accessibility.Public false i
getAccessibility AccessControlLevel.Public false i


/// Is an attribute with a given name?
Expand Down Expand Up @@ -282,8 +287,8 @@ let isInterface typeDef =

let checkAccessibility currentAccessibility = function
| Some(SynAccess.Public) | None -> currentAccessibility
| Some(SynAccess.Private) -> Accessibility.Private
| Some(SynAccess.Internal) -> Accessibility.Internal
| Some(SynAccess.Private) -> AccessControlLevel.Private
| Some(SynAccess.Internal) -> AccessControlLevel.Internal

let isModule (moduleKind:SynModuleOrNamespaceKind) =
match moduleKind with
Expand All @@ -302,11 +307,11 @@ let isImplicitModule (SynModuleOrNamespace.SynModuleOrNamespace(longIdent, _, mo
// TODO: does SynModuleOrNamespaceKind.AnonModule replace this check?
isModule moduleKind && longIdent |> List.forall (fun x -> zeroLengthRange x.idRange)

type GetIdents<'t> = Accessibility -> SynPat -> 't []
type GetIdents<'t> = AccessControlLevel -> SynPat -> 't []

/// Recursively get all identifiers from pattern using provided getIdents function and collect them into array.
/// accessibility parameter is passed to getIdents, and can be narrowed down along the way (see checkAccessibility).
let rec getPatternIdents<'t> (accessibility:Accessibility) (getIdents:GetIdents<'t>) argsAreParameters (pattern:SynPat) =
let rec getPatternIdents<'t> (accessibility:AccessControlLevel) (getIdents:GetIdents<'t>) argsAreParameters (pattern:SynPat) =
match pattern with
| SynPat.LongIdent(_, _, _, args, access, _) ->
let identAccessibility = checkAccessibility accessibility access
Expand All @@ -321,11 +326,11 @@ let rec getPatternIdents<'t> (accessibility:Accessibility) (getIdents:GetIdents<
| SynArgPats.NamePatPairs(pats, _) ->
pats
|> List.toArray
|> Array.collect (snd >> getPatternIdents Accessibility.Private getIdents false)
|> Array.collect (snd >> getPatternIdents AccessControlLevel.Private getIdents false)
| SynArgPats.Pats(pats) ->
pats
|> List.toArray
|> Array.collect (getPatternIdents Accessibility.Private getIdents false)
|> Array.collect (getPatternIdents AccessControlLevel.Private getIdents false)

// Only check if expecting args as parameters e.g. function - otherwise is a DU pattern.
if hasNoArgs || argsAreParameters then
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,12 +43,12 @@ let private getIdentifiers (args:AstNodeRuleParams) =
let accessibility = getAccessibility args.SyntaxArray args.NodeIndex
getPatternIdents accessibility (getValueOrFunctionIdents args.CheckInfo) true pattern
| Member | Property ->
getPatternIdents Accessibility.Private getMemberIdents true pattern
getPatternIdents AccessControlLevel.Private getMemberIdents true pattern
| _ -> Array.empty
else
Array.empty
| AstNode.Expression(SynExpr.ForEach(_, _, true, pattern, _, _, _)) ->
getPatternIdents Accessibility.Private (getValueOrFunctionIdents args.CheckInfo) false pattern
getPatternIdents AccessControlLevel.Private (getValueOrFunctionIdents args.CheckInfo) false pattern
| _ -> Array.empty

let rule config =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ let private getValueOrFunctionIdents typeChecker accessibility pattern =
match List.tryLast longIdent.Lid with
| Some ident when not (isActivePattern ident) && singleIdentifier ->
let checkNotUnionCase = checkNotUnionCase ident
if accessibility = Accessibility.Private then
if accessibility = AccessControlLevel.Private then
(ident, ident.idText, Some checkNotUnionCase)
|> Array.singleton
else
Expand All @@ -31,7 +31,7 @@ let private getValueOrFunctionIdents typeChecker accessibility pattern =
let private getIdentifiers (args:AstNodeRuleParams) =
match args.AstNode with
| AstNode.Expression(SynExpr.ForEach(_, _, true, pattern, _, _, _)) ->
getPatternIdents Accessibility.Private (getValueOrFunctionIdents args.CheckInfo) false pattern
getPatternIdents AccessControlLevel.Private (getValueOrFunctionIdents args.CheckInfo) false pattern
| AstNode.Binding(SynBinding(access, _, _, _, attributes, _, valData, pattern, _, _, _, _)) ->
if not (isLiteral attributes) then
match identifierTypeFromValData valData with
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ let private getValueOrFunctionIdents typeChecker isPublic pattern =
match List.tryLast longIdent.Lid with
| Some ident when singleIdentifier ->
let checkNotUnionCase = checkNotUnionCase ident
if isPublic = Accessibility.Public && isNotActivePattern ident then
if isPublic = AccessControlLevel.Public && isNotActivePattern ident then
(ident, ident.idText, Some checkNotUnionCase)
|> Array.singleton
else
Expand All @@ -35,7 +35,7 @@ let private getValueOrFunctionIdents typeChecker isPublic pattern =
let private getIdentifiers (args:AstNodeRuleParams) =
match args.AstNode with
| AstNode.Expression(SynExpr.ForEach(_, _, true, pattern, _, _, _)) ->
getPatternIdents Accessibility.Private (getValueOrFunctionIdents args.CheckInfo) false pattern
getPatternIdents AccessControlLevel.Private (getValueOrFunctionIdents args.CheckInfo) false pattern
| AstNode.Binding(SynBinding(_, _, _, _, attributes, _, valData, pattern, _, _, _, _)) ->
if not (isLiteral attributes || isExtern attributes) then
match identifierTypeFromValData valData with
Expand Down