Remove suggestions logic

Following the removal of the `-suggestions` flag the code being removed
here is no longer used. Hence, this removal cleans up the code base.

This also updates the Contributing Guidelines accordingly.

Signed-off-by: Eric Cornelissen <ericornelissen@gmail.com>

CONTRIBUTING.md

@@ -55,11 +55,10 @@ tests to the `rules_test.go` file. The details depend on whether it's a rule for
 with `uses:`) or for other steps, but defining the rule is the same regardless.
 
 To define a rule you need to create an instance of the `rule` type. This involves giving the rule an
-id, title, and description as well as a function to extract what needs to be analyzed and a function
-that builds a suggestion for fixing a violation. The id, title, and description are simple text
-values. The extraction function needs to return a string to be analyzed for a given `JobStep`. The
-suggestion functions needs to return a suggestion string for a given `Violation`. Lastly, you need
-to add the rule to the `RULES.md` documentation file in the same format as the `--explain` output.
+id, title, and description as well as a function to extract what needs to be analyzed. The id,
+title, and description are simple text values. The extraction function needs to return a string to
+be analyzed for a given `JobStep`. Lastly, you need to add the rule to the `RULES.md` documentation
+file in the same format as the `--explain` output.
 
 Note that if multiple things could be checked for one action or step construct, they should be
 defined as separate rules.
@@ -78,6 +77,6 @@ the `stepRule` type. You also need to add the rule to the `stepRules` slice.
 ### Testing
 
 Every new rule needs to be tested. The rule id, title, and description are tested automatically. The
-`appliesTo`, `extractFrom`, and `suggestion` functions require dedicated unit tests. For this, it is
-recommended to follow the lead of the tests for existing rules. Additionally, a test case should be
-added to the `test/rules.txtar` test file.
+`appliesTo` and `extractFrom` functions require dedicated unit tests. For this, it is recommended to
+follow the lead of the tests for existing rules. Additionally, a test case should be added to the
+`test/rules.txtar` test file.

rules.go

@@ -25,7 +25,6 @@ import (
 
 type rule struct {
 	extractFrom func(step *JobStep) string
-	suggestion  func(violation *Violation) string
 	fix         func(violation *Violation) []fix
 	id          string
 	title       string
@@ -83,7 +82,6 @@ it can be made safer by converting it into:
 		extractFrom: func(step *JobStep) string {
 			return step.With["script"]
 		},
-		suggestion: suggestJavaScriptEnv,
 	},
 }
 
@@ -101,9 +99,6 @@ upgrade the action to a non-vulnerable version.`,
 		extractFrom: func(step *JobStep) string {
 			return step.With["summary"]
 		},
-		suggestion: func(_ *Violation) string {
-			return "    1. Upgrade to a non-vulnerable version, see GHSA-4xqx-pqpj-9fqw"
-		},
 	},
 }
 
@@ -121,9 +116,6 @@ mitigate this, upgrade the action to a non-vulnerable version.`,
 		extractFrom: func(step *JobStep) string {
 			return step.With["tag"]
 		},
-		suggestion: func(_ *Violation) string {
-			return "    1. Upgrade to a non-vulnerable version, see GHSA-hgx2-4pp9-357g"
-		},
 	},
 }
 
@@ -141,9 +133,6 @@ this, upgrade the action to a non-vulnerable version.`,
 		extractFrom: func(step *JobStep) string {
 			return step.With["sha"]
 		},
-		suggestion: func(_ *Violation) string {
-			return "    1. Upgrade to a non-vulnerable version, see v1.2.0 release notes"
-		},
 	},
 }
 
@@ -179,7 +168,6 @@ it can be made safer by converting it into:
 		extractFrom: func(step *JobStep) string {
 			return step.With["issue-close-message"]
 		},
-		suggestion: suggestJavaScriptLiteralEnv,
 		fix: func(violation *Violation) []fix {
 			var step JobStep
 			switch source := (violation.source).(type) {
@@ -238,7 +226,6 @@ it can be made safer by converting it into:
 		extractFrom: func(step *JobStep) string {
 			return step.With["pr-close-message"]
 		},
-		suggestion: suggestJavaScriptLiteralEnv,
 	},
 }
 
@@ -276,7 +263,6 @@ it can be made safer by converting it into:
 		extractFrom: func(step *JobStep) string {
 			return step.With["cmd"]
 		},
-		suggestion: suggestShellEnv,
 	},
 }
 
@@ -333,7 +319,6 @@ it can be made safer by converting it into:
 		extractFrom: func(step *JobStep) string {
 			return step.Run
 		},
-		suggestion: suggestShellEnv,
 	},
 }
 
@@ -371,17 +356,6 @@ func Explain(ruleId string) (string, error) {
 	return explanation, nil
 }
 
-// Suggestion returns a suggestion for the violation.
-func Suggestion(violation *Violation) (string, error) {
-	ruleId := violation.RuleId
-	r, err := findRule(ruleId)
-	if err != nil {
-		return "", err
-	}
-
-	return r.suggestion(violation), nil
-}
-
 // Fix produces a set of fixes to address the violation if possible. If the return value is nil the
 // violation cannot be fixed automatically.
 func Fix(violation *Violation) ([]fix, error) {
@@ -452,33 +426,6 @@ func fixReplaceIn(s, old, new string) fix {
 	}
 }
 
-func suggestJavaScriptEnv(violation *Violation) string {
-	return suggestUseEnv(violation.Problem, "process.env.", "")
-}
-
-func suggestJavaScriptLiteralEnv(violation *Violation) string {
-	return suggestUseEnv(violation.Problem, "${process.env.", "}")
-}
-
-func suggestShellEnv(violation *Violation) string {
-	return suggestUseEnv(violation.Problem, "$", "")
-}
-
-func suggestUseEnv(problem, pre, post string) string {
-	var sb strings.Builder
-
-	name := getVariableNameForExpression(problem)
-	replacement := pre + name + post
-
-	sb.WriteString(fmt.Sprintf("    1. Set `%s: %s` in the step's `env` map", name, problem))
-	sb.WriteRune('\n')
-	sb.WriteString(fmt.Sprintf("    2. Replace all occurrences of `%s` by `%s`", problem, replacement))
-	sb.WriteRune('\n')
-	sb.WriteString("       (make sure to keep the behavior of the script the same)")
-
-	return sb.String()
-}
-
 func getVariableNameForExpression(expression string) string {
 	name := expression[strings.LastIndex(expression, ".")+1:]
 	name = strings.TrimRight(name, "}")