Differentiate raw and inferred 1-part source string (#3)

* Differentiate raw and inferred 1-part source string

* Update provider.go

Co-authored-by: Alisdair McDiarmid <alisdair@users.noreply.github.com>

Co-authored-by: Alisdair McDiarmid <alisdair@users.noreply.github.com>

README.md

@@ -11,7 +11,7 @@ via [`hashicorp/terraform-exec`](https://github.com/hashicorp/terraform-exec).
 ## Example
 
 ```go
-p, err := ParseProviderSourceString("hashicorp/aws")
+p, err := ParseRawProviderSourceString("hashicorp/aws")
 if err != nil {
 	// deal with error
 }
@@ -23,41 +23,52 @@ if err != nil {
 // }
 ```
 
-Please note that the `ParseProviderSourceString` is **NOT** equipped
-to deal with legacy addresses such as `aws`. Such address will be parsed
-as if provider belongs to `hashicorp` namespace in the public Registry.
-
 ## Legacy address
 
 A legacy address is by itself (without more context) ambiguous.
 For example `aws` may represent either the official `hashicorp/aws`
 or just any custom-built provider called `aws`.
 
-If you know the address was produced by Terraform <=0.12 and/or that you're
-dealing with a legacy address, the following sequence of steps should be taken.
+Such ambiguous address can be produced by Terraform `<=0.12`. You can
+just use `ImpliedProviderForUnqualifiedType` if you know for sure
+the address was produced by an affected version.
+
+If you do not have that context you should parse the string via
+`ParseRawProviderSourceString` and then check `addr.IsLegacy()`.
 
-(optional) Parse such legacy address by `NewLegacyProvider(name)`.
+### What to do with a legacy address?
 
 Ask the Registry API whether and where the provider was moved to
 
 (`-` represents the legacy, basically unknown namespace)
 
 ```sh
 # grafana (redirected to its own namespace)
-$ curl -s https://registry.terraform.io/v1/providers/-/grafana/versions | jq .moved_to
+$ curl -s https://registry.terraform.io/v1/providers/-/grafana/versions | jq '(.id, .moved_to)'
+"terraform-providers/grafana"
 "grafana/grafana"
 
 # aws (provider without redirection)
-$ curl -s https://registry.terraform.io/v1/providers/-/aws/versions | jq .moved_to
+$ curl -s https://registry.terraform.io/v1/providers/-/aws/versions | jq '(.id, .moved_to)'
+"hashicorp/aws"
 null
 ```
 
 Then:
 
- - Use `ParseProviderSourceString` for the _new_ (`moved_to`) address of any _moved_ provider (e.g. `grafana/grafana`).
- - Use `ImpliedProviderForUnqualifiedType` for any other provider (e.g. `aws`)
-   - Depending on context `terraform` may also be parsed by `ParseProviderSourceString`,
-   	 which assumes `hashicorp/terraform` provider. Read more about this provider below.
+ - Reparse the _new_ address (`moved_to`) of any _moved_ provider (e.g. `grafana/grafana`) via `ParseRawProviderSourceString`
+ - Reparse the full address (`id`) of any other provider (e.g. `hashicorp/aws`)
+
+Depending on context (legacy) `terraform` may need to be parsed separately.
+Read more about this provider below.
+
+If for some reason you cannot ask the Registry API you may also use
+`ParseAndInferProviderSourceString` which assumes that any legacy address
+(including `terraform`) belongs to the `hashicorp` namespace.
+
+If you cache results (which you should), ensure you have invalidation
+mechanism in place because target (migrated) namespace may change.
+Hard-coding migrations anywhere in code is strongly discouraged.
 
 ### `terraform` provider
 
@@ -74,5 +85,4 @@ Alternatively you may just treat the address as the builtin provider,
 i.e. assume all of its logic including schema is contained within
 Terraform Core.
 
-In such case you should use `ImpliedProviderForUnqualifiedType(typeName)`,
-as the function makes such assumption.
+In such case you should just use `NewBuiltInProvider("terraform")`.

provider.go

@@ -219,50 +219,30 @@ func (pt Provider) Equals(other Provider) bool {
 	return pt == other
 }
 
-// ParseProviderSourceString parses the source attribute and returns a provider.
+// ParseRawProviderSourceString parses the source attribute and returns a provider.
 // This is intended primarily to parse the FQN-like strings returned by
 // terraform-config-inspect.
 //
 // The following are valid source string formats:
 // 		name
 // 		namespace/name
 // 		hostname/namespace/name
-func ParseProviderSourceString(str string) (Provider, error) {
+//
+// "name"-only format is parsed as -/name (i.e. legacy namespace)
+// requiring further identification of the namespace via Registry API
+func ParseRawProviderSourceString(str string) (Provider, error) {
 	var ret Provider
-
-	// split the source string into individual components
-	parts := strings.Split(str, "/")
-	if len(parts) == 0 || len(parts) > 3 {
-		return ret, &ParserError{
-			Summary: "Invalid provider source string",
-			Detail:  `The "source" attribute must be in the format "[hostname/][namespace/]name"`,
-		}
-	}
-
-	// check for an invalid empty string in any part
-	for i := range parts {
-		if parts[i] == "" {
-			return ret, &ParserError{
-				Summary: "Invalid provider source string",
-				Detail:  `The "source" attribute must be in the format "[hostname/][namespace/]name"`,
-			}
-		}
-	}
-
-	// check the 'name' portion, which is always the last part
-	givenName := parts[len(parts)-1]
-	name, err := ParseProviderPart(givenName)
+	parts, err := parseSourceStringParts(str)
 	if err != nil {
-		return ret, &ParserError{
-			Summary: "Invalid provider type",
-			Detail:  fmt.Sprintf(`Invalid provider type %q in source %q: %s"`, givenName, str, err),
-		}
+		return ret, err
 	}
+
+	name := parts[len(parts)-1]
 	ret.Type = name
 	ret.Hostname = DefaultRegistryHost
 
 	if len(parts) == 1 {
-		return NewDefaultProvider(parts[0]), nil
+		return NewLegacyProvider(name), nil
 	}
 
 	if len(parts) >= 2 {
@@ -352,10 +332,68 @@ func ParseProviderSourceString(str string) (Provider, error) {
 	return ret, nil
 }
 
-// MustParseProviderSourceString is a wrapper around ParseProviderSourceString that panics if
+// ParseAndInferProviderSourceString parses the source attribute and returns a provider.
+// This is intended primarily to parse the FQN-like strings returned by
+// terraform-config-inspect.
+//
+// The following are valid source string formats:
+// 		name
+// 		namespace/name
+// 		hostname/namespace/name
+//
+// "name" format is assumed to be hashicorp/name
+func ParseAndInferProviderSourceString(str string) (Provider, error) {
+	var ret Provider
+	parts, err := parseSourceStringParts(str)
+	if err != nil {
+		return ret, err
+	}
+
+	if len(parts) == 1 {
+		return NewDefaultProvider(parts[0]), nil
+	}
+
+	return ParseRawProviderSourceString(str)
+}
+
+func parseSourceStringParts(str string) ([]string, error) {
+	// split the source string into individual components
+	parts := strings.Split(str, "/")
+	if len(parts) == 0 || len(parts) > 3 {
+		return nil, &ParserError{
+			Summary: "Invalid provider source string",
+			Detail:  `The "source" attribute must be in the format "[hostname/][namespace/]name"`,
+		}
+	}
+
+	// check for an invalid empty string in any part
+	for i := range parts {
+		if parts[i] == "" {
+			return nil, &ParserError{
+				Summary: "Invalid provider source string",
+				Detail:  `The "source" attribute must be in the format "[hostname/][namespace/]name"`,
+			}
+		}
+	}
+
+	// check the 'name' portion, which is always the last part
+	givenName := parts[len(parts)-1]
+	name, err := ParseProviderPart(givenName)
+	if err != nil {
+		return nil, &ParserError{
+			Summary: "Invalid provider type",
+			Detail:  fmt.Sprintf(`Invalid provider type %q in source %q: %s"`, givenName, str, err),
+		}
+	}
+	parts[len(parts)-1] = name
+
+	return parts, nil
+}
+
+// MustParseRawProviderSourceString is a wrapper around ParseRawProviderSourceString that panics if
 // it returns an error.
-func MustParseProviderSourceString(str string) Provider {
-	result, err := ParseProviderSourceString(str)
+func MustParseRawProviderSourceString(str string) Provider {
+	result, err := ParseRawProviderSourceString(str)
 	if err != nil {
 		panic(err)
 	}