PR review

dotnet · Nov 3, 2024 · 3adf463 · 3adf463
1 parent 698e3f2
commit 3adf463
Show file tree

Hide file tree

Showing 8 changed files with 88 additions and 80 deletions.
diff --git a/src/Compiler/Checking/Expressions/CheckExpressions.fs b/src/Compiler/Checking/Expressions/CheckExpressions.fs
@@ -10642,7 +10642,7 @@ and TcMatchClause cenv inputTy (resultTy: OverallTy) env isFirst tpenv synMatchC
     let (SynMatchClause(synPat, synWhenExprOpt, synResultExpr, patm, spTgt, trivia)) = synMatchClause
 
     let isTrueMatchClause =
-        if trivia.BarRange.IsSome && trivia.ArrowRange.IsSome then
+        if synMatchClause.IsTrueMatchClause then
             TcTrueMatchClause.Yes
         else
             TcTrueMatchClause.No

diff --git a/src/Compiler/Checking/Expressions/CheckSequenceExpressions.fs b/src/Compiler/Checking/Expressions/CheckSequenceExpressions.fs
@@ -270,9 +270,9 @@ let TcSequenceExpression (cenv: TcFileState) env tpenv comp (overallTy: OverallT
 
             let tclauses, tpenv =
                 (tpenv, clauses)
-                ||> List.mapFold (fun tpenv (SynMatchClause(pat, cond, innerComp, _, sp, trivia)) ->
+                ||> List.mapFold (fun tpenv (SynMatchClause(pat, cond, innerComp, _, sp, trivia) as clause) ->
                     let isTrueMatchClause =
-                        if trivia.BarRange.IsSome && trivia.ArrowRange.IsSome then
+                        if clause.IsTrueMatchClause then
                             TcTrueMatchClause.Yes
                         else
                             TcTrueMatchClause.No
@@ -319,9 +319,9 @@ let TcSequenceExpression (cenv: TcFileState) env tpenv comp (overallTy: OverallT
             // Compile the pattern twice, once as a filter with all succeeding targets returning "1", and once as a proper catch block.
             let clauses, tpenv =
                 (tpenv, withList)
-                ||> List.mapFold (fun tpenv (SynMatchClause(pat, cond, innerComp, m, sp, trivia)) ->
+                ||> List.mapFold (fun tpenv (SynMatchClause(pat, cond, innerComp, m, sp, trivia) as clause) ->
                     let isTrueMatchClause =
-                        if trivia.BarRange.IsSome && trivia.ArrowRange.IsSome then
+                        if clause.IsTrueMatchClause then
                             TcTrueMatchClause.Yes
                         else
                             TcTrueMatchClause.No

diff --git a/src/Compiler/Checking/NameResolution.fs b/src/Compiler/Checking/NameResolution.fs
@@ -3392,19 +3392,17 @@ let rec ResolvePatternLongIdentPrim sink (ncenv: NameResolver) fullyQualified wa
             | true, res when not newDef -> ResolveUnqualifiedItem ncenv nenv m res
             | _ ->
                 // Single identifiers in patterns - variable bindings
-                let supportsWarnOnUpperIdentifiersInPatterns = ncenv.g.langVersion.SupportsFeature(LanguageFeature.DontWarnOnUppercaseIdentifiersInBindingPatterns)
-                if (supportsWarnOnUpperIdentifiersInPatterns && not newDef && System.Char.ToLowerInvariant id.idText[0] <> id.idText[0])
+                let supportsDontWarnOnUppercaseIdentifiers = ncenv.g.langVersion.SupportsFeature(LanguageFeature.DontWarnOnUppercaseIdentifiersInBindingPatterns)
+                let isUpperCaseIdentifier = (not newDef && System.Char.ToLowerInvariant id.idText[0] <> id.idText[0])
+                if (supportsDontWarnOnUppercaseIdentifiers && isUpperCaseIdentifier)
                 then
                     match warnOnUpper with
                     | WarnOnUpperUnionCaseLabel -> warning(UpperCaseIdentifierInPattern m)
                     | WarnOnUpperVariablePatterns
                     | AllIdsOK -> ()
                 else
-                    if not newDef
-                        // HACK: This is an historical hack that seems to related the use country and language codes, which are very common in codebases
-                        && id.idText.Length >= 3
-                        && System.Char.ToLowerInvariant id.idText[0] <> id.idText[0]
-                    then
+                    // HACK: This is an historical hack that seems to related the use country and language codes, which are very common in codebases
+                    if isUpperCaseIdentifier && id.idText.Length >= 3 then
                         match warnOnUpper with
                         | WarnOnUpperUnionCaseLabel
                         | WarnOnUpperVariablePatterns ->  warning(UpperCaseIdentifierInPattern m)

diff --git a/src/Compiler/SyntaxTree/SyntaxTree.fs b/src/Compiler/SyntaxTree/SyntaxTree.fs
@@ -1047,6 +1047,10 @@ type SynMatchClause =
             | None -> e.Range
             | Some x -> unionRanges e.Range x.Range
 
+    member this.IsTrueMatchClause =
+        let (SynMatchClause(trivia = trivia)) = this
+        trivia.BarRange.IsSome && trivia.ArrowRange.IsSome
+
     member this.Range =
         match this with
         | SynMatchClause(range = m) -> m

diff --git a/src/Compiler/SyntaxTree/SyntaxTree.fsi b/src/Compiler/SyntaxTree/SyntaxTree.fsi
@@ -1177,6 +1177,8 @@ type SynMatchClause =
     /// Gets the syntax range of part of this construct
     member RangeOfGuardAndRhs: range
 
+    member IsTrueMatchClause: bool
+
     /// Gets the syntax range of this construct
     member Range: range