Range of signature module includes attribute for signature but not for implementation file.

[nojaf]

Please provide a succinct description of the issue.

Consider the following code and matching AST:

[<  Foo  >]
module Bar

let s : string = "s"

Implementation AST

ImplFile
  (ParsedImplFileInput
     ("tmp.fsx", true, QualifiedNameOfFile Bar$fsx, [], [],
      [SynModuleOrNamespace
         ([Bar], false, NamedModule,
          [Let(...)],
          PreXmlDoc ((2,6), FSharp.Compiler.XmlDoc+XmlDocCollector),
          [{ Attributes =
              [{ TypeName = LongIdentWithDots ([Foo], [])
                 ArgExpr = Const (Unit, tmp.fsx (1,4--1,7) IsSynthetic=false)
                 Target = None
                 AppliesToGetterAndSetter = false
                 Range = tmp.fsx (1,4--1,7) IsSynthetic=false }]
             Range = tmp.fsx (1,0--1,11) IsSynthetic=false }], None,
          tmp.fsx (2,0--4,20) IsSynthetic=false)], (true, true)))

and

[<  Foo  >]
module Bar

val s : string

Signature file

SigFile
  (ParsedSigFileInput
     ("tmp.fsi", QualifiedNameOfFile Bar, [], [],
      [SynModuleOrNamespaceSig
         ([Bar], false, NamedModule,
          [Val(...)],
          PreXmlDoc ((2,6), FSharp.Compiler.XmlDoc+XmlDocCollector),
          [{ Attributes =
              [{ TypeName = LongIdentWithDots ([Foo], [])
                 ArgExpr = Const (Unit, tmp.fsi (1,4--1,7) IsSynthetic=false)
                 Target = None
                 AppliesToGetterAndSetter = false
                 Range = tmp.fsi (1,4--1,7) IsSynthetic=false }]
             Range = tmp.fsi (1,0--1,11) IsSynthetic=false }], None,
          tmp.fsi (1,0--4,14) IsSynthetic=false)]))

Expected behavior

I'm not sure what the expected behavior should be in this case.
SynModuleOrNamespace starts at line 2 while SynModuleOrNamespaceSig starts at line 1.

Actual behavior

The range of the attributes is not included in SynModuleOrNamespace yet it is for SynModuleOrNamespaceSig.

Known workarounds

I calculate the full range using an extension:

type SynModuleOrNamespace with
    member mn.FullRange : Range =
        match mn with
        | SynModuleOrNamespace(attribs = h::_) ->
            Range.mkRange mn.Range.FileName h.Range.Start mn.Range.End
        | _ -> mn.Range

I'm ok with this workaround but would like both AST nodes to show the exact same behaviour.
Please let me know what is expected in this case. I'm interested in submitting a PR.

Related information

FCS 39.

[cartermp]

I think we'd definitely take a PR to bring this up to your expectations.

[nojaf]

Ok, to be on the safe side. Could you state which type of file shows the correct behaviour?
Is ImpFile correct or is SigFile?

[auduchinok]

The range should include attributes. I've fixed it for some of other things in previous PRs that were accepted. 🙂

[cartermp]

Sorry, missed this. I agree with @auduchinok here.

[nojaf]

Ok, could someone explain when an AST node should contain the range of attributes and when not?
I've encountered more examples where it is not included than when it is.

Example:

[<FooBar>]
type Meh =
    class
    end

[TypeDefn
                 (ComponentInfo
                    ([{ Attributes =
                         [{ TypeName = LongIdentWithDots ([FooBar], [])
                            ArgExpr =
                             Const (Unit, tmp.fsx (1,2--1,8) IsSynthetic=false)
                            Target = None
                            AppliesToGetterAndSetter = false
                            Range = tmp.fsx (1,2--1,8) IsSynthetic=false }]
                        Range = tmp.fsx (1,0--1,10) IsSynthetic=false }], [], [],
                     [Meh],
                     PreXmlDoc ((2,8), FSharp.Compiler.XmlDoc+XmlDocCollector),
                     false, None, tmp.fsx (2,5--2,8) IsSynthetic=false),
                  ObjectModel
                    (TyconClass, [], tmp.fsx (3,4--4,7) IsSynthetic=false), [],
                  tmp.fsx (2,5--4,7) IsSynthetic=false)]

Are the rules just different for modules?
Any chance the rulebook is published somewhere?

[auduchinok]

In my opinion they should be included everywhere.