Skip to content
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
17 changes: 14 additions & 3 deletions src/absil/illib.fs
Original file line number Diff line number Diff line change
Expand Up @@ -503,9 +503,20 @@ module String =
let uppercase (s: string) =
s.ToUpperInvariant()

let isUpper (s: string) =
s.Length >= 1 && Char.IsUpper s.[0] && not (Char.IsLower s.[0])

// Scripts that distinguish between upper and lower case (bicameral) DU Discriminators and Active Pattern identifiers are required to start with an upper case character.
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Scripts - this comment implies that this change is only relevant for F# scripts. It's also relevant to regular .fs files, yes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@cartermp , scripts in this case means written form of a language.

// For valid identifiers where the case of the identifier can not be determined because there is no upper and lower case we will allow DU Discriminators and upper case characters
// to be used. This means that developers using unicameral scripts such as hindi, are not required to prefix these identifiers with an Upper case latin character.
//
let isLeadingIdentifierCharacterUpperCase (s:string) =
let isUpperCaseCharacter c =
// if IsUpper and IsLower return the same value, then we can't tell if it's upper or lower case, so ensure it is a letter
// otherwise it is bicameral, so must be upper case
let isUpper = Char.IsUpper c
if isUpper = Char.IsLower c then Char.IsLetter c
else isUpper

s.Length >= 1 && isUpperCaseCharacter s.[0]

let capitalize (s: string) =
if s.Length = 0 then s
else uppercase s.[0..0] + s.[ 1.. s.Length - 1 ]
Expand Down
6 changes: 3 additions & 3 deletions src/fsharp/TypeChecker.fs
Original file line number Diff line number Diff line change
Expand Up @@ -5233,7 +5233,7 @@ and TcPatBindingName cenv env id ty isMemberThis vis1 topValData (inlineFlag, de
let name = id.idText
match values.TryGetValue name with
| true, value ->
if not (String.IsNullOrEmpty name) && Char.IsLower(name.[0]) then
if not (String.IsNullOrEmpty name) && not (String.isLeadingIdentifierCharacterUpperCase name) then
match env.eNameResEnv.ePatItems.TryGetValue name with
| true, Item.Value vref when vref.LiteralValue.IsSome ->
warning(Error(FSComp.SR.checkLowercaseLiteralBindingInPattern name, id.idRange))
Expand Down Expand Up @@ -12775,7 +12775,7 @@ module TcRecdUnionAndEnumDeclarations = begin
errorR(Error(FSComp.SR.tcUnionCaseNameConflictsWithGeneratedType(name, "Tags"), id.idRange))

CheckNamespaceModuleOrTypeName cenv.g id
if not (String.isUpper name) && name <> opNameCons && name <> opNameNil then
if not (String.isLeadingIdentifierCharacterUpperCase name) && name <> opNameCons && name <> opNameNil then
errorR(NotUpperCaseConstructor(id.idRange))

let ValidateFieldNames (synFields: SynField list, tastFields: RecdField list) =
Expand Down Expand Up @@ -15174,7 +15174,7 @@ module TcExceptionDeclarations =

let TcExnDefnCore_Phase1A cenv env parent (SynExceptionDefnRepr(Attributes synAttrs, UnionCase(_, id, _, _, _, _), _, doc, vis, m)) =
let attrs = TcAttributes cenv env AttributeTargets.ExnDecl synAttrs
if not (String.isUpper id.idText) then errorR(NotUpperCaseConstructor m)
if not (String.isLeadingIdentifierCharacterUpperCase id.idText) then errorR(NotUpperCaseConstructor m)
let vis, cpath = ComputeAccessAndCompPath env None m vis None parent
let vis = TcRecdUnionAndEnumDeclarations.CombineReprAccess parent vis
CheckForDuplicateConcreteType env (id.idText + "Exception") id.idRange
Expand Down
6 changes: 3 additions & 3 deletions src/fsharp/pars.fsy
Original file line number Diff line number Diff line change
Expand Up @@ -3050,7 +3050,7 @@ atomicPattern:

| atomicPatternLongIdent %prec prec_atompat_pathop
{ let vis, lidwd = $1
if not (isNilOrSingleton lidwd.Lid) || (let c = (List.head lidwd.Lid).idText.[0] in Char.IsUpper(c) && not (Char.IsLower c))
if not (isNilOrSingleton lidwd.Lid) || String.isLeadingIdentifierCharacterUpperCase (List.head lidwd.Lid).idText
then mkSynPatMaybeVar lidwd vis (lhs parseState)
else mkSynPatVar vis (List.head lidwd.Lid) }

Expand Down Expand Up @@ -5290,8 +5290,8 @@ operatorName:

/* One part of an active pattern name */
activePatternCaseName:
| IDENT
{ if not (String.isUpper $1) then reportParseErrorAt (rhs parseState 1) (FSComp.SR.parsActivePatternCaseMustBeginWithUpperCase());
| IDENT
{ if not (String.isLeadingIdentifierCharacterUpperCase _1) then reportParseErrorAt (rhs parseState 1) (FSComp.SR.parsActivePatternCaseMustBeginWithUpperCase());
if ($1.IndexOf('|') <> -1) then reportParseErrorAt (rhs parseState 1) (FSComp.SR.parsActivePatternCaseContainsPipe());
$1 }

Expand Down
Original file line number Diff line number Diff line change
@@ -1,18 +1,20 @@
// #Regression #Conformance #PatternMatching #ActivePatterns
// Verify error if Active Patterns do not start with an upper case letter
//<Expects id="FS0623" status="error" span="(11,7)">Active pattern case identifiers must begin with an uppercase letter</Expects>
//<Expects id="FS0623" status="error" span="(11,16)">Active pattern case identifiers must begin with an uppercase letter</Expects>
//<Expects id="FS0623" status="error" span="(12,7)">Active pattern case identifiers must begin with an uppercase letter</Expects>
//<Expects id="FS0623" status="error" span="(13,10)">Active pattern case identifiers must begin with an uppercase letter</Expects>
//<Expects id="FS0623" status="error" span="(14,7)">Active pattern case identifiers must begin with an uppercase letter</Expects>
//<Expects id="FS0624" status="error" span="(15,7)">The '\|' character is not permitted in active pattern case identifiers</Expects>
//<Expects id="FS0624" status="error" span="(16,9)">The '\|' character is not permitted in active pattern case identifiers</Expects>
//<Expects id="FS0623" status="error" span="(12,16)">Active pattern case identifiers must begin with an uppercase letter</Expects>
//<Expects id="FS0623" status="error" span="(13,7)">Active pattern case identifiers must begin with an uppercase letter</Expects>
//<Expects id="FS0623" status="error" span="(14,10)">Active pattern case identifiers must begin with an uppercase letter</Expects>
//<Expects id="FS0623" status="error" span="(15,7)">Active pattern case identifiers must begin with an uppercase letter</Expects>
//<Expects id="FS0624" status="error" span="(16,7)">The '\|' character is not permitted in active pattern case identifiers</Expects>
//<Expects id="FS0624" status="error" span="(17,9)">The '\|' character is not permitted in active pattern case identifiers</Expects>
//<Expects id="FS0623" status="error" span="(18,7)">Active pattern case identifiers must begin with an uppercase letter</Expects>

let (|positive|negative|) n = if n < 0 then positive else negative
let (|`` A``|) (x:int) = x
let (|B1|``+B2``|) (x:int) = if x = 0 then OneA else ``+B2``
let (|`` C``|_|) (x:int) = if x = 0 then Some(x) else None
Copy link
Member

Choose a reason for hiding this comment

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

Would it be better to preserve these cases? So, if produced errors change one day it'll be reflected here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@auduchinok once we do the is letter thingy they will be errors again :-).

Copy link
Member

@auduchinok auduchinok May 15, 2020

Choose a reason for hiding this comment

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

Yes, it'll be OK. :)

What I wanted to say was if there is a test for some particular behavior that is being changed it could be good to keep the test just in case it might be changed again (even accidentally, e.g. if fixing an issue produced by the initial change). It's probably just a good thing to record how the particular behavior is being changed anyway.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @auduchinok mate, I understood mate, was messin' with ya :-)

let (|``D|E``|F|) (x:int) = if x = 0 then D elif x = 1 then E else F
let (|G|``H||I``|) (x:int) = if x = 0 then G elif x = 1 then H else ``|I``
let (|_J|) (x:int) = _J

exit 1
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,17 @@
type Foo =
| A of int
| B of string * int

let test x =
match x with
| A(1) | B(_,1) -> 1
| A(2) | B(_,2) -> 2
| B(_, _) -> -1
| A(_) -> -2

if test (A(1)) <> 1 then exit 1
if test (B("",1)) <> 1 then exit 1

if test (A(2)) <> 2 then exit 1
if test (B("",2)) <> 2 then exit 1

Expand Down
4 changes: 2 additions & 2 deletions tests/fsharpqa/Source/Globalization/Hindi.fs
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ module पिछले =

// DU
type ख़तरxनाक =
| Uअलगाववादी // There's no uppercase/lowercase in Hindi, so I'm adding a latin char
| Aमिलती of ख़तरनाक
| अलगाववादी // There's no uppercase/lowercase in Hindi, ensure that a Hindi character will suffice to start the DU case name
| मिलती of ख़तरनाक
| X

// Record
Expand Down