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.

Sign up for GitHub

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

Print ParamArray attribute instead of 'params' #192

Closed
wants to merge 6 commits into from
Closed

Print ParamArray attribute instead of 'params' #192

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
3fde68d
Print ParamArray attribute instead of 'params'
dungpa Jan 31, 2015
a8972fe
Fix up tests
dungpa Jan 31, 2015
adb4fa2
Minor refactoring
dungpa Feb 4, 2015
93bd099
Fix language service unit tests
dungpa Feb 4, 2015
e7f654f
Update FSharpQA tests
dungpa Feb 5, 2015
301dfaa
Update test listing
dungpa Feb 5, 2015
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
70 changes: 45 additions & 25 deletions src/fsharp/NicePrint.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,37 @@ module private PrintUtilities =
| (x:: rest) -> [ resultFunction x (layoutFunction x -- leftL (match rest.Length with 1 -> FSComp.SR.nicePrintOtherOverloads1() | n -> FSComp.SR.nicePrintOtherOverloadsN(n))) ]
| _ -> []

/// Layout a reference to a type
let layoutTyconRefImpl isAttribute (denv: DisplayEnv) (tcref:TyconRef) =
let demangled =
let name =
if denv.includeStaticParametersInTypeNames then
tcref.DisplayNameWithStaticParameters
elif tcref.DisplayName = tcref.DisplayNameWithStaticParameters then
tcref.DisplayName // has no static params
else
tcref.DisplayName+"<...>" // shorten
if isAttribute then
defaultArg (String.tryDropSuffix name "Attribute") name
else name
let tyconTextL = wordL demangled
if denv.shortTypeNames then
tyconTextL
else
let path = demangledPathOfCompPath tcref.CompilationPath
let path =
Copy link
Contributor

Choose a reason for hiding this comment

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

used same bound name path, use another instead of replace path

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are subjective opinions on value shadowing. Since the code is moved from another place in the file, I will keep it as is.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, that's fine. FWIW there is plenty of shadowing in the compiler in any case

Copy link
Contributor

Choose a reason for hiding this comment

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

@dungpa I have not seen that code was moved only.
btw @dsyme about shadowing (asking, like for <| on code review ) is it usually ok? or we should avoit it for new code ( not moved code )

if denv.includeStaticParametersInTypeNames then
path
else
path |> List.map (fun s -> let i = s.IndexOf(',')
if i <> -1 then s.Substring(0,i)+"<...>" // apparently has static params, shorten
else s)
let pathText = trimPathByDisplayEnv denv path
if pathText = "" then tyconTextL else leftL pathText ^^ tyconTextL

let layoutTyconAttribute (denv: DisplayEnv) (tcref:TyconRef) =
squareAngleL (layoutTyconRefImpl true denv tcref)

module private PrintIL =

open Microsoft.FSharp.Compiler.AbstractIL.IL
Expand Down Expand Up @@ -185,7 +216,9 @@ module private PrintIL =
// Layout an unnamed argument
| _, None, _ -> leftL ":"
// Layout a named argument
| true, Some nm,_ -> wordL "params" ^^ leftL (nm+":")
| true, Some nm,_ ->
let typ = denv.g.attrib_ParamArrayAttribute.TyconRef
layoutTyconAttribute denv typ ^^ leftL (nm + ":")
| false, Some nm,_ -> leftL (nm+":")
preL ^^ (layoutILType denv ilTyparSubst p.Type)

Expand Down Expand Up @@ -549,28 +582,7 @@ module private PrintTypes =
| Internal,Private -> wordL "private" ++ itemL // print modifier, since more specific than context
| _ -> itemL

/// Layout a reference to a type
let layoutTyconRef (denv: DisplayEnv) (tcref:TyconRef) =
let demangled = if denv.includeStaticParametersInTypeNames then
tcref.DisplayNameWithStaticParameters
elif tcref.DisplayName = tcref.DisplayNameWithStaticParameters then
tcref.DisplayName // has no static params
else
tcref.DisplayName+"<...>" // shorten
let tyconTextL = wordL demangled
if denv.shortTypeNames then
tyconTextL
else
let path = demangledPathOfCompPath tcref.CompilationPath
let path =
if denv.includeStaticParametersInTypeNames then
path
else
path |> List.map (fun s -> let i = s.IndexOf(',')
if i <> -1 then s.Substring(0,i)+"<...>" // apparently has static params, shorten
else s)
let pathText = trimPathByDisplayEnv denv path
if pathText = "" then tyconTextL else leftL pathText ^^ tyconTextL
let layoutTyconRef denv tycon = layoutTyconRefImpl false denv tycon

/// Layout the flags of a member
let layoutMemberFlags memFlags =
Expand Down Expand Up @@ -918,7 +930,13 @@ module private PrintTypes =
layoutTypeWithInfoAndPrec denv env 2 ty
// Layout a named argument
| Some id,_,isParamArray,_ ->
leftL ((if isParamArray then "params " else "") + id.idText) ^^ sepL ":" ^^ layoutTypeWithInfoAndPrec denv env 2 ty
let prefix =
if isParamArray then
let typ = denv.g.attrib_ParamArrayAttribute.TyconRef
layoutTyconAttribute denv typ ^^ leftL id.idText
else
leftL id.idText
prefix ^^ sepL ":" ^^ layoutTypeWithInfoAndPrec denv env 2 ty

let delimitReturnValue = if denv.useColonForReturnType then ":" else "->"

Expand Down Expand Up @@ -1147,7 +1165,9 @@ module InfoMemberPrinting =
outputTy denv os pty;
// Layout a named argument
| true, Some nm,_,_ ->
bprintf os "params %s: " nm
let typ = denv.g.attrib_ParamArrayAttribute.TyconRef
layoutTyconAttribute denv typ |> bufferL os
bprintf os " %s: " nm
outputTy denv os pty
| false, Some nm,_,_ ->
bprintf os "%s: " nm
Expand Down
11 changes: 11 additions & 0 deletions tests/fsharpqa/Source/Printing/ParamArrayInSignatures.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// #Regression #NoMT #Printing
Copy link
Contributor

Choose a reason for hiding this comment

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

  • I would rename this to .fsx since it's being run via FSI
  • As is, piping this into FSI actually results in FSI exit code of 1, which causes the test to fail.
    • You need to add #q at the end to get a 0 exit code

#light

// pretty printing signatures with params arguments
//<Expects status=success>type Heterogeneous =</Expects>
//<Expects status=success> class</Expects>
//<Expects status=success> static member Echo : \[<System.ParamArray>\] args:obj \[\] -> obj \[\]</Expects>
//<Expects status=success> end</Expects>

type Heterogeneous =
static member Echo([<System.ParamArray>] args: obj[]) = args
Copy link
Contributor

Choose a reason for hiding this comment

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

no newline at eof

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

1 change: 1 addition & 0 deletions tests/fsharpqa/Source/Printing/env.lst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,3 +32,4 @@ ReqENU SOURCE=LazyValues03NetFx4.fsx COMPILE_ONLY=1 SCFLAGS="--nologo" FSIMODE=P

SOURCE=WidthForAFormatter.fs # WidthForAFormatter.fs
SOURCE=ToStringOnCollections.fs # ToStringOnCollections.fs
SOURCE=ParamArrayInSignatures.fs SCFLAGS="--nologo" COMPILE_ONLY=1 FSIMODE=PIPE # ParamArrayInSignatures.fs