Added no-http-verbs-in-path rule.

Also make it a little easier to update tests when rules are added.

functions/functions.go

@@ -96,6 +96,7 @@ func MapBuiltinFunctions() Functions {
 		funcs["oasDocumentSchema"] = openapi_functions.OASSchema{}
 		funcs["oasAPIServers"] = openapi_functions.APIServers{}
 		funcs["noAmbiguousPaths"] = openapi_functions.AmbiguousPaths{}
+		funcs["noVerbsInPath"] = openapi_functions.VerbsInPaths{}
 		funcs["oasOpErrorResponse"] = openapi_functions.Operation4xResponse{}
 
 	})

functions/functions_test.go

@@ -7,5 +7,5 @@ import (
 
 func TestMapBuiltinFunctions(t *testing.T) {
 	funcs := MapBuiltinFunctions()
-	assert.Len(t, funcs.GetAllFunctions(), 40)
+	assert.Len(t, funcs.GetAllFunctions(), 41)
 }

functions/openapi/no_http_verbs_in_path.go

@@ -0,0 +1,66 @@
+// Copyright 2022 Dave Shanley / Quobix
+// SPDX-License-Identifier: MIT
+
+package openapi
+
+import (
+	"fmt"
+	"github.com/daveshanley/vacuum/model"
+	"github.com/pb33f/libopenapi/utils"
+	"gopkg.in/yaml.v3"
+	"strings"
+)
+
+// VerbsInPaths Checks to make sure that no HTTP verbs have been used
+type VerbsInPaths struct {
+}
+
+// GetSchema returns a model.RuleFunctionSchema defining the schema of the VerbsInPath rule.
+func (vp VerbsInPaths) GetSchema() model.RuleFunctionSchema {
+	return model.RuleFunctionSchema{Name: "noVerbsInPath"}
+}
+
+// RunRule will execute the VerbsInPath rule, based on supplied context and a supplied []*yaml.Node slice.
+func (vp VerbsInPaths) RunRule(nodes []*yaml.Node, context model.RuleFunctionContext) []model.RuleFunctionResult {
+
+	if len(nodes) <= 0 {
+		return nil
+	}
+
+	var results []model.RuleFunctionResult
+
+	ops := context.Index.GetPathsNode()
+
+	var opPath string
+
+	if ops != nil {
+		for i, op := range ops.Content {
+			if i%2 == 0 {
+				opPath = op.Value
+				continue
+			}
+			path := fmt.Sprintf("$.paths.%s", opPath)
+			containsVerb, verb := checkPath(opPath)
+			if containsVerb {
+				results = append(results, model.RuleFunctionResult{
+					Message:   fmt.Sprintf("Path `%s` contains an HTTP Verb `%s`", opPath, verb),
+					StartNode: op,
+					EndNode:   op,
+					Path:      path,
+					Rule:      context.Rule,
+				})
+			}
+		}
+	}
+	return results
+}
+
+func checkPath(path string) (bool, string) {
+	segs := strings.Split(path, "/")[1:]
+	for _, seg := range segs {
+		if utils.IsHttpVerb(strings.ToLower(seg)) {
+			return true, seg
+		}
+	}
+	return false, ""
+}

functions/openapi/no_http_verbs_in_path_test.go

@@ -0,0 +1,62 @@
+package openapi
+
+import (
+	"github.com/daveshanley/vacuum/model"
+	"github.com/pb33f/libopenapi/index"
+	"github.com/pb33f/libopenapi/utils"
+	"github.com/stretchr/testify/assert"
+	"gopkg.in/yaml.v3"
+	"testing"
+)
+
+func TestVerbsInPaths_GetSchema(t *testing.T) {
+	def := VerbsInPaths{}
+	assert.Equal(t, "noVerbsInPath", def.GetSchema().Name)
+}
+
+func TestVerbsInPaths_RunRule(t *testing.T) {
+	def := VerbsInPaths{}
+	res := def.RunRule(nil, model.RuleFunctionContext{})
+	assert.Len(t, res, 0)
+}
+
+func TestVerbsInPaths_Success(t *testing.T) {
+
+	yml := `openapi: 3.0.0
+paths:
+  '/oh/no/post/man':
+    get:
+      summary: bad path
+  '/this/one/is/OK':
+    get:
+      summary: good path
+  '/i/am/going/to/get/failed':
+    get:
+      summary: not a good one.
+  '/will/you/patch/my/code':
+    get:
+      summary: this is also doomed
+  '/put/my/cake/away':
+    get:
+      summary: another bad one.
+  '/this/is/fine':
+    get:
+      summary: all done squire`
+	path := "$"
+
+	var rootNode yaml.Node
+	yaml.Unmarshal([]byte(yml), &rootNode)
+
+	nodes, _ := utils.FindNodes([]byte(yml), path)
+
+	rule := buildOpenApiTestRuleAction(path, "verbsInPath", "", nil)
+	ctx := buildOpenApiTestContext(model.CastToRuleAction(rule.Then), nil)
+	ctx.Rule = &rule
+	ctx.Index = index.NewSpecIndex(&rootNode)
+
+	def := VerbsInPaths{}
+	res := def.RunRule(nodes, ctx)
+
+	assert.Len(t, res, 4)
+
+}

rulesets/rule_fixes.go

@@ -143,6 +143,9 @@ const (
 		"are the same thing. Make sure every path and the variables used are unique and do conflict with one another. Check the ordering of variables " +
 		"and the naming of path segments."
 
+	noVerbsInPathFix string = "When HTTP verbs (get/post/put etc) are used in path segments, it muddies the semantics of REST and creates a confusing and " +
+		"inconsistent experience. It's highly recommended that verbs are not used in path segments. Replace those HTTP verbs with more meaningful nouns."
+
 	operationsErrorResponseFix string = "Make sure each operation defines at least one 4xx error response. 4xx Errors are " +
 		"used to inform clients they are using the API incorrectly, with bad input, or malformed requests. An API with no errors" +
 		"defined is really hard to navigate."

rulesets/ruleset_functions.go

@@ -1148,6 +1148,26 @@ func NoAmbiguousPaths() *model.Rule {
 	}
 }
 
+// GetNoVerbsInPathRule will check all paths to make sure not HTTP verbs have been used as a segment.
+func GetNoVerbsInPathRule() *model.Rule {
+	return &model.Rule{
+		Name:         "Paths cannot contain HTTP verbs as segments",
+		Id:           noVerbsInPath,
+		Formats:      model.AllFormats,
+		Description:  "Path segments must not contain an HTTP verb",
+		Given:        "$",
+		Resolved:     false,
+		Recommended:  true,
+		RuleCategory: model.RuleCategories[model.CategoryOperations],
+		Type:         style,
+		Severity:     warn,
+		Then: model.RuleAction{
+			Function: "noVerbsInPath",
+		},
+		HowToFix: noVerbsInPathFix,
+	}
+}
+
 // GetOperationErrorResponseRule will return the rule for checking for a 4xx response defined in operations.
 func GetOperationErrorResponseRule() *model.Rule {
 	return &model.Rule{

rulesets/rulesets.go

@@ -29,6 +29,7 @@ const (
 	allPaths      = "$.paths[*]"
 	allOperations = "[?(@.get || @.post || @.put || @.patch || @.delete || @.trace || @.options || @.head)]"
 
+	noVerbsInPath                     = "no-http-verbs-in-path"
 	noAmbiguousPaths                  = "no-ambiguous-paths"
 	operationErrorResponse            = "operation-4xx-response"
 	operationSuccessResponse          = "operation-success-response"
@@ -300,6 +301,7 @@ func generateDefaultOpenAPIRuleSet() *RuleSet {
 	rules[oas3ValidSchemaExample] = GetOAS3ExamplesRule()
 	rules[oas2ValidSchemaExample] = GetOAS2ExamplesRule()
 	rules[noAmbiguousPaths] = NoAmbiguousPaths()
+	rules[noVerbsInPath] = GetNoVerbsInPathRule()
 	rules[operationErrorResponse] = GetOperationErrorResponseRule()
 	rules[oas2Schema] = GetOAS2SchemaRule()
 	rules[oas3Schema] = GetOAS3SchemaRule()

rulesets/rulesets_test.go

@@ -5,11 +5,14 @@ import (
 	"testing"
 )
 
+var totalRules = 52
+var totalRecommendedRules = 41
+
 func TestBuildDefaultRuleSets(t *testing.T) {
 
 	rs := BuildDefaultRuleSets()
 	assert.NotNil(t, rs.GenerateOpenAPIDefaultRuleSet())
-	assert.Len(t, rs.GenerateOpenAPIDefaultRuleSet().Rules, 51)
+	assert.Len(t, rs.GenerateOpenAPIDefaultRuleSet().Rules, totalRules)
 
 }
 
@@ -121,10 +124,10 @@ func TestRuleSet_GetConfiguredRules_All(t *testing.T) {
 	// read spec and parse to dashboard.
 	rs := BuildDefaultRuleSets()
 	ruleSet := rs.GenerateOpenAPIDefaultRuleSet()
-	assert.Len(t, ruleSet.Rules, 51)
+	assert.Len(t, ruleSet.Rules, totalRules)
 
 	ruleSet = rs.GenerateOpenAPIRecommendedRuleSet()
-	assert.Len(t, ruleSet.Rules, 40)
+	assert.Len(t, ruleSet.Rules, totalRecommendedRules)
 
 }
 
@@ -140,7 +143,7 @@ rules:
 	def := BuildDefaultRuleSets()
 	rs, _ := CreateRuleSetFromData([]byte(yaml))
 	override := def.GenerateRuleSetFromSuppliedRuleSet(rs)
-	assert.Len(t, override.Rules, 40)
+	assert.Len(t, override.Rules, totalRecommendedRules)
 	assert.Len(t, override.RuleDefinitions, 1)
 }
 
@@ -173,7 +176,7 @@ rules:
 	def := BuildDefaultRuleSets()
 	rs, _ := CreateRuleSetFromData([]byte(yaml))
 	override := def.GenerateRuleSetFromSuppliedRuleSet(rs)
-	assert.Len(t, override.Rules, 51)
+	assert.Len(t, override.Rules, totalRules)
 	assert.Len(t, override.RuleDefinitions, 1)
 }
 
@@ -189,7 +192,7 @@ rules:
 	def := BuildDefaultRuleSets()
 	rs, _ := CreateRuleSetFromData([]byte(yaml))
 	override := def.GenerateRuleSetFromSuppliedRuleSet(rs)
-	assert.Len(t, override.Rules, 39)
+	assert.Len(t, override.Rules, totalRecommendedRules-1)
 	assert.Len(t, override.RuleDefinitions, 1)
 }
 
@@ -205,7 +208,7 @@ rules:
 	def := BuildDefaultRuleSets()
 	rs, _ := CreateRuleSetFromData([]byte(yaml))
 	override := def.GenerateRuleSetFromSuppliedRuleSet(rs)
-	assert.Len(t, override.Rules, 40)
+	assert.Len(t, override.Rules, totalRecommendedRules)
 	assert.Equal(t, "hint", override.Rules["operation-success-response"].Severity)
 }
 
@@ -265,7 +268,7 @@ rules:
 	def := BuildDefaultRuleSets()
 	rs, _ := CreateRuleSetFromData([]byte(yaml))
 	newrs := def.GenerateRuleSetFromSuppliedRuleSet(rs)
-	assert.Len(t, newrs.Rules, 52)
+	assert.Len(t, newrs.Rules, totalRules+1)
 	assert.Equal(t, true, newrs.Rules["fish-cakes"].Recommended)
 	assert.Equal(t, "yummy sea food", newrs.Rules["fish-cakes"].Description)
 
@@ -290,7 +293,7 @@ rules:
 	def := BuildDefaultRuleSets()
 	rs, _ := CreateRuleSetFromData([]byte(yaml))
 	repl := def.GenerateRuleSetFromSuppliedRuleSet(rs)
-	assert.Len(t, repl.Rules, 51)
+	assert.Len(t, repl.Rules, totalRules)
 	assert.Equal(t, true, repl.Rules["info-contact"].Recommended)
 	assert.Equal(t, "yummy sea food", repl.Rules["info-contact"].Description)
 
@@ -315,7 +318,7 @@ rules:
 	def := BuildDefaultRuleSets()
 	rs, _ := CreateRuleSetFromData([]byte(yaml))
 	repl := def.GenerateRuleSetFromSuppliedRuleSet(rs)
-	assert.Len(t, repl.Rules, 51)
+	assert.Len(t, repl.Rules, totalRules)
 	assert.Equal(t, true, repl.Rules["info-contact"].Recommended)
 	assert.Equal(t, "yummy sea food", repl.Rules["info-contact"].Description)