Limit parameters count

[delebedev]

Very early version of #415.
Implementation depends on answer here: jpsim/SourceKitten#151.

[jpsim]

You could avoid the hardcoded string literal by doing return SwiftDeclarationKind(rawValue: key) == . VarParameter instead.

[delebedev]

nice, had no idea there were constants for that :)

[jpsim]

I'm excited to see you getting started on this! Let me know if you have any questions moving forward.

[delebedev]

@jpsim assuming my code example here jpsim/SourceKitten#151 do you think there are "good" ™️ way to extract function parameters from AST?

I'm inclining to simply get function declaration string and parse it manually. Moreover I have to do it anyway because of default parameters (they are not represented in SourceKit AFAIK)

[jpsim]

@jpsim assuming my code example here jpsim/SourceKitten#151 (comment) do you think there are "good" way to extract function parameters from AST?

Yeah, it's spot on. You're using an ASTRule and only continuing the validation process when substructures have .VarParameters. 👍

[delebedev]

@jpsim but apparently it triggers false positive for this simple snippet:

private func foo(dictionary: [Int: Int]) -> Int {
    dictionary.flatMap { foo in return foo }
    return 1
}

produces 1 level-nested structure which will count foo parameters and return 2 not 1:

  "key.substructure" : [
    {
      "key.kind" : "source.lang.swift.decl.function.free",
      "key.offset" : 22,
      "key.nameoffset" : 27,
      "key.namelength" : 27,
      "key.bodyoffset" : 63,
      "key.bodylength" : 73,
      "key.accessibility" : "source.lang.swift.accessibility.private",
      "key.substructure" : [
        {
          "key.kind" : "source.lang.swift.decl.var.parameter",
          "key.offset" : 31,
          "key.nameoffset" : 0,
          "key.namelength" : 0,
          "key.length" : 10,
          "key.typename" : "[Int: Int]",
          "key.name" : "dictionary"
        },
        {
          "key.kind" : "source.lang.swift.decl.var.parameter",
          "key.offset" : 103,
          "key.nameoffset" : 0,
          "key.namelength" : 0,
          "key.length" : 3,
          "key.typename" : "foo",
          "key.name" : "foo"
        }
      ],
      "key.name" : "foo(_:)",
      "key.length" : 115
    }
  ],
  "key.offset" : 0,
  "key.diagnostic_stage" : "source.diagnostic.stage.swift.parse",
  "key.length" : 138
}

[jpsim]

In your example, foo is a parameter. It's a parameter to the closure. If you want to filter just parameters to functions, you can filter .VarParameters that have a key.offset between its enclosing function's key.offset and key.bodyoffset.

In your example, you'd only end up with dictionary as parameters for functions because its key.offset is 17, which falls between its parent function's key.offset of 8 and key.bodyoffset of 49. Whereas foo isn't considered because its key.offset is 71, which is outside the 8..<49 range.

[delebedev]

@jpsim thanks for this 👍 I had a feeling I'm missing something

[delebedev]

usage of for loop is intentional (not very functional 😃) : it allows to return early in case method reached end of current function declaration

[delebedev]

@jpsim ready to be reviewed!

[norio-nomura]

Could you please use Location(file: file, byteOffset: offset) instead of Location(file: file, characterOffset: offset)?
Because dictionary["key.offset"] is byte offset.

[jpsim]

Typo: {Parameters

[jpsim]

The implementation here looks great! I'd like us to really nail down the rule name though.

"Parameters List Length" and "Parameter List Length" seem to be used interchangeably. We're not really measuring the string length here, but rather the number of parameters.

I think "Function Parameter Count" might be a more appropriate name.

Otherwise like I said, the implementation looks spot on (as far as I can tell reveling this from my phone 😉).

[delebedev]

@jpsim agreed with naming, changed

[jpsim]

Thanks @garnett! I just had a few minor changes to add, so I'll merge in #459.

Really nice work on this! Not too many people have ventured into writing ASTRule based rules so far, so if you learned anything that you think might be useful to add in the contributing.md documentation for others looking to write rules, please let me know!

[norio-nomura]

👏