Linter: Ignore false positives in gradually-deprecated check (hashico…

…rp#24393)

Before this commit, the `scripts/run-gradually-deprecated.sh` check took
the list of files that differed between `HEAD` (often the tip of a PR
branch) and the current tip of `origin/main`, and checked `HEAD`'s
version for use of a set of idioms marked `deprecated`. Unfortunately,
this is pretty prone to false positives, where the use of a deprecated
idiom is fixed in `origin/main` but not yet included in `HEAD`; making
the check fail, as the deprecated use is present in `HEAD`, although it
would not be present post-merge, as `HEAD` would be merged on top of the
existing fix in `origin/main`.

Specifically, I ran into this problem three times in a row (and
counting!) on another PR, which is my motivation for this fix:

hashicorp#24071

Those all followed the pattern mentioned above:

1. I make a commit, containing no deprecated uses; merge gating checks
   don't yet run because they haven't been approved
2. Some deprecated use is fixed on main
3. The merge checks are approved by a maintainer, run, and fail because
   of step 2
4. I see that something's changed and rebase on the new changes; `goto`
   step 1.

From the `git` man page:

    git diff [<options>] [--merge-base] <commit> [--] [<path>...]
        This form is to view the changes you have in your working tree
        relative to the named <commit>. You can use HEAD to compare it
        with the latest commit, or a branch name to compare with the
        tip of a different branch.

        If --merge-base is given, instead of using <commit>, use the
        merge base of <commit> and HEAD.  git diff --merge-base A is
        equivalent to git diff $(git merge-base A HEAD).

So in essence, this causes the script to only check changes from this PR
rather than this PR's changes plus the inverse of every other commit to
`main`.

This other change would also be equivalent, but is in my opinion less
clear as to what it does:

```diff
diff --git a/scripts/run-gradually-deprecated.sh b/scripts/run-gradually-deprecated.sh
index 993814b35a..f2a6b20785 100755
--- a/scripts/run-gradually-deprecated.sh
+++ b/scripts/run-gradually-deprecated.sh
@@ -9 +9 @@ function runGraduallyDeprecatedFunctions {
-  IFS=$'\n' read -r -d '' -a flist < <(git diff --diff-filter=AMRC origin/main --name-only)
+  IFS=$'\n' read -r -d '' -a flist < <(git diff --diff-filter=AMRC origin/main... --name-only)
```

Here's the testing I did, that confirms this would behave the way I
expect:

```
% git show --stat # Here I'm on the commit that caused me problems in the above-mentioned PR
commit e7e6203 (HEAD)
Author: Chandler Swift <[email protected]>
Date:   Wed Nov 29 12:26:20 2023 -0600

    provider: don't attempt to register unavailable RPs

    PR hashicorp#23380 addressed issue hashicorp#21785, but with that fix, when an unavailable
    resource provider was found, it would still be added to the list of
    providers to be registered, and then fail upon attempted registration.

    This skips registration for providers which are neither registered nor
    unregistered.

    Co-Authored-By: Jeff Smith <[email protected]>
    Co-Authored-By: Ryan Wozney <[email protected]>

 internal/resourceproviders/requiring_registration.go | 1 +
 1 file changed, 1 insertion(+)
% git diff --diff-filter=AMRC origin/main --name-only | head # The diff shows many files
CHANGELOG.md
go.mod
go.sum
internal/resourceproviders/requiring_registration.go
internal/services/authorization/role_definition_data_source.go
internal/services/authorization/role_definition_data_source_test.go
internal/services/automation/automation_module_resource.go
internal/services/automation/automation_runbook_resource.go
internal/services/automation/automation_runbook_resource_test.go
internal/services/automation/registration.go
% git diff --diff-filter=AMRC origin/main --name-only | wc -l # _maaaaany_ files
480
% git diff --diff-filter=AMRC origin/main --name-only --merge-base # but with `--merge-base`, just the one we expect
internal/resourceproviders/requiring_registration.go
% ./scripts/run-gradually-deprecated.sh # And sure enough, the unmodified script fails on an unrelated file:
==> Checking for use of gradually deprecated functions...
internal/services/iothub/iothub_resource.go:73:		Create: resourceIotHubCreateUpdate,
New Resources should no longer use combined CreateUpdate methods, please
split these into two separate Create and Update methods.

Existing resources can continue to use combined CreateUpdate methods
for the moment - but over time these will be split into separate Create and
Update methods.

% vim ./scripts/run-gradually-deprecated.sh # add `--merge-base`
% git diff
diff --git a/scripts/run-gradually-deprecated.sh b/scripts/run-gradually-deprecated.sh
index 993814b35a..f2a6b20785 100755
--- a/scripts/run-gradually-deprecated.sh
+++ b/scripts/run-gradually-deprecated.sh
@@ -6,7 +6,7 @@
 function runGraduallyDeprecatedFunctions {
   echo "==> Checking for use of gradually deprecated functions..."

-  IFS=$'\n' read -r -d '' -a flist < <(git diff --diff-filter=AMRC origin/main --name-only)
+  IFS=$'\n' read -r -d '' -a flist < <(git diff --diff-filter=AMRC origin/main --name-only --merge-base)

   for f in "${flist[@]}"; do
     # require resources to be imported is now hard-coded on - but only checking for additions
% ./scripts/run-gradually-deprecated.sh # Now it doesn't false-positive!
==> Checking for use of gradually deprecated functions...
==> Checking for use of deprecated functions...
% # Here I add a line that _should_ cause a failure:
% echo features.ShouldResourcesBeImported >> internal/resourceproviders/requiring_registration.go
% git diff internal/resourceproviders/requiring_registration.go
diff --git a/internal/resourceproviders/requiring_registration.go b/internal/resourceproviders/requiring_registration.go
index 65c0f08bf1..b6e8927100 100644
--- a/internal/resourceproviders/requiring_registration.go
+++ b/internal/resourceproviders/requiring_registration.go
@@ -31,3 +31,4 @@ func DetermineWhichRequiredResourceProvidersRequireRegistration(requiredResource

        return &requiringRegistration, nil
 }
+features.ShouldResourcesBeImported
% ./scripts/run-gradually-deprecated.sh # ...which, indeed, it does!
==> Checking for use of gradually deprecated functions...
internal/resourceproviders/requiring_registration.go:34:features.ShouldResourcesBeImported
The Feature Flag for 'ShouldResourcesBeImported' will be deprecated in the future
and shouldn't be used in new resources - please remove new usages of the
'ShouldResourcesBeImported' function from these changes - since this is now enabled
by default.

In the future this function will be marked as Deprecated - however it's not for
the moment to not conflict with open Pull Requests.
```

Co-authored-by: Jeff Smith <[email protected]>

scripts/run-gradually-deprecated.sh

@@ -6,7 +6,7 @@
 function runGraduallyDeprecatedFunctions {
   echo "==> Checking for use of gradually deprecated functions..."
 
-  IFS=$'\n' read -r -d '' -a flist < <(git diff --diff-filter=AMRC origin/main --name-only)
+  IFS=$'\n' read -r -d '' -a flist < <(git diff --diff-filter=AMRC origin/main --name-only --merge-base)
 
   for f in "${flist[@]}"; do
     # require resources to be imported is now hard-coded on - but only checking for additions