Add warning for calls to provider without doc or fields.

WARNINGS.md

@@ -48,6 +48,7 @@ Warning categories supported by buildifier's linter:
   * [`package-on-top`](#package-on-top)
   * [`positional-args`](#positional-args)
   * [`print`](#print)
+  * [`provider-params`](#provider-params)
   * [`redefined-variable`](#redefined-variable)
   * [`repository-name`](#repository-name)
   * [`return-value`](#return-value)
@@ -796,6 +797,38 @@ may never see the warning.
 
 --------------------------------------------------------------------------------
 
+## <a name="provider-params"></a>Calls to 'provider' should specify a list of fields and a documentation
+
+  * Category name: `provider-params`
+  * Automatic fix: no
+
+Calls to `provider` should specify a documentation string and a list of fields:
+
+```python
+fooInfo = provider(
+    doc = "Some documentation.",
+    fields = ["field1", "field2"],
+)
+```
+
+Fields should also be documented when needed:
+
+```python
+provider(
+    doc = "Some documentation.",
+    fields = {
+        "field1": "Documentation for field1",
+        "field2': "Documentation for field2",
+    })
+``
+
+Note that specifying a list of fields is a breaking change. It is an error if a
+call to the provider uses undeclared fields.
+
+See the [documentation for providers](https://docs.bazel.build/versions/master/skylark/lib/globals.html#provider).
+
+--------------------------------------------------------------------------------
+
 ## <a name="redefined-variable"></a>Variable has already been defined
 
   * Category name: `redefined-variable`

warn/warn.go

@@ -144,6 +144,7 @@ var FileWarningMap = map[string]func(f *build.File) []*LinterFinding{
 	"package-name":              packageNameWarning,
 	"package-on-top":            packageOnTopWarning,
 	"print":                     printWarning,
+	"provider-params":           providerParamsWarning,
 	"redefined-variable":        redefinedVariableWarning,
 	"repository-name":           repositoryNameWarning,
 	"rule-impl-return":          ruleImplReturnWarning,

warn/warn_bazel_api.go

@@ -1008,3 +1008,33 @@ func keywordPositionalParametersWarning(f *build.File) []*LinterFinding {
 
 	return findings
 }
+
+func providerParamsWarning(f *build.File) []*LinterFinding {
+	if f.Type != build.TypeBzl {
+		return nil
+	}
+
+	var findings []*LinterFinding
+	build.Walk(f, func(expr build.Expr, stack []build.Expr) {
+		call, ok := isFunctionCall(expr, "provider")
+		if !ok {
+			return
+		}
+
+		_, _, fields := getParam(call.List, "fields")
+		_, _, doc := getParam(call.List, "doc")
+		// doc can also be the first positional argument
+		hasPositional := false
+		if len(call.List) > 0 {
+			if _, ok := call.List[0].(*build.AssignExpr); !ok {
+				hasPositional = true
+			}
+		}
+		if fields == nil || (doc == nil && !hasPositional) {
+			findings = append(findings, makeLinterFinding(call,
+				`Calls to 'provider' should specify a list of fields and a documentation:\n`+
+					`  provider("description", fields = [...])`))
+		}
+	})
+	return findings
+}

warn/warn_bazel_api_test.go

@@ -2,8 +2,9 @@ package warn
 
 import (
 	"fmt"
-	"github.com/bazelbuild/buildtools/tables"
 	"testing"
+
+	"github.com/bazelbuild/buildtools/tables"
 )
 
 func TestAttrConfigurationWarning(t *testing.T) {
@@ -749,3 +750,13 @@ getattr(
 		`:12: Keyword parameter "default" for "getattr" should be positional.`,
 	}, scopeEverywhere)
 }
+
+func TestProvider(t *testing.T) {
+	checkFindings(t, "provider-params", `provider(doc = "doc", fields = [])`, []string{}, scopeBzl)
+	checkFindings(t, "provider-params", `provider("doc", fields = [])`, []string{}, scopeBzl)
+
+	err := `1: Calls to 'provider' should specify a list of fields and a documentation:\n  provider("description", fields = [...])`
+	checkFindings(t, "provider-params", `provider(fields = [])`, []string{err}, scopeBzl)
+	checkFindings(t, "provider-params", `provider(doc = "doc")`, []string{err}, scopeBzl)
+	checkFindings(t, "provider-params", `p = provider()`, []string{err}, scopeBzl)
+}