Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes for false positives in unused-components rule #532

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions functions/openapi/unused_component.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@ func (uc UnusedComponent) RunRule(nodes []*yaml.Node, context model.RuleFunction

var results []model.RuleFunctionResult

// extract all references, and every single component
allRefs := context.Index.GetAllReferences()
// extract all references, and every single component, recursively
allRefs := context.Document.GetRolodex().GetAllReferences()
schemas := context.Index.GetAllComponentSchemas()
responses := context.Index.GetAllResponses()
parameters := context.Index.GetAllParameters()
Expand All @@ -49,7 +49,7 @@ func (uc UnusedComponent) RunRule(nodes []*yaml.Node, context model.RuleFunction
securitySchemes := context.Index.GetAllSecuritySchemes()
links := context.Index.GetAllLinks()
callbacks := context.Index.GetAllCallbacks()
mappedRefs := context.Index.GetMappedReferences()
mappedRefs := context.Document.GetRolodex().GetAllMappedReferences()

// extract securityRequirements from swagger. These are not mapped as they are not $refs
// so, we need to map them as if they were.
Expand Down
151 changes: 44 additions & 107 deletions functions/openapi/unused_component_test.go
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
package openapi

import (
"fmt"
"github.com/daveshanley/vacuum/model"
"github.com/pb33f/libopenapi"
"github.com/pb33f/libopenapi/datamodel"
"github.com/pb33f/libopenapi/index"
"github.com/pb33f/libopenapi/utils"
"github.com/stretchr/testify/assert"
"gopkg.in/yaml.v3"
"strings"
"testing"
)

Expand All @@ -23,7 +26,9 @@ func TestUnusedComponent_RunRule(t *testing.T) {

func TestUnusedComponent_RunRule_Success(t *testing.T) {

yml := `paths:
yml := `
openapi: 3.0.0
paths:
/naughty/{puppy}:
parameters:
- $ref: '#/components/parameters/Chewy'
Expand All @@ -46,22 +51,7 @@ components:
in: query
name: chewy`

path := "$"

var rootNode yaml.Node
mErr := yaml.Unmarshal([]byte(yml), &rootNode)
assert.NoError(t, mErr)

nodes, _ := utils.FindNodes([]byte(yml), path)

rule := buildOpenApiTestRuleAction(path, "unused_component", "", nil)
ctx := buildOpenApiTestContext(model.CastToRuleAction(rule.Then), nil)
config := index.CreateOpenAPIIndexConfig()
ctx.Index = index.NewSpecIndexWithConfig(&rootNode, config)

def := UnusedComponent{}
res := def.RunRule(nodes, ctx)

res := setupUnusedComponentsTestContext(t, yml)
daveshanley marked this conversation as resolved.
Show resolved Hide resolved
assert.Len(t, res, 0)
}

Expand All @@ -85,24 +75,7 @@ paths:
security:
- sessionAuth: []`

path := "$"

var rootNode yaml.Node
mErr := yaml.Unmarshal([]byte(yml), &rootNode)
assert.NoError(t, mErr)

nodes, _ := utils.FindNodes([]byte(yml), path)

rule := buildOpenApiTestRuleAction(path, "unused_component", "", nil)
ctx := buildOpenApiTestContext(model.CastToRuleAction(rule.Then), nil)
config := index.CreateOpenAPIIndexConfig()
ctx.Index = index.NewSpecIndexWithConfig(&rootNode, config)
info, _ := datamodel.ExtractSpecInfo([]byte(yml))
ctx.SpecInfo = info
ctx.Rule = &rule

def := UnusedComponent{}
res := def.RunRule(nodes, ctx)
res := setupUnusedComponentsTestContext(t, yml)

assert.Len(t, res, 0)
}
Expand All @@ -118,30 +91,15 @@ components:
securitySchemes:
SomeSecurity:
description: A secure way to do things and stuff.`
path := "$"

var rootNode yaml.Node
mErr := yaml.Unmarshal([]byte(yml), &rootNode)
assert.NoError(t, mErr)

nodes, _ := utils.FindNodes([]byte(yml), path)

rule := buildOpenApiTestRuleAction(path, "unused_component", "", nil)
ctx := buildOpenApiTestContext(model.CastToRuleAction(rule.Then), nil)
config := index.CreateOpenAPIIndexConfig()
ctx.Index = index.NewSpecIndexWithConfig(&rootNode, config)
info, _ := datamodel.ExtractSpecInfo([]byte(yml))
ctx.SpecInfo = info

def := UnusedComponent{}
res := def.RunRule(nodes, ctx)

res := setupUnusedComponentsTestContext(t, yml)
assert.Len(t, res, 0)
}

func TestUnusedComponent_RunRule_Success_Fail_TwoMissing_Two_Undefined(t *testing.T) {

yml := `parameters:
yml := `
openapi: 3.0.1
parameters:
Chewy:
description: chewy
in: query
Expand All @@ -166,28 +124,15 @@ components:
Kitty:
$ref: '#/components/schemas/Puppy' `

path := "$"

var rootNode yaml.Node
mErr := yaml.Unmarshal([]byte(yml), &rootNode)
assert.NoError(t, mErr)

nodes, _ := utils.FindNodes([]byte(yml), path)

rule := buildOpenApiTestRuleAction(path, "unused_component", "", nil)
ctx := buildOpenApiTestContext(model.CastToRuleAction(rule.Then), nil)
config := index.CreateOpenAPIIndexConfig()
ctx.Index = index.NewSpecIndexWithConfig(&rootNode, config)

def := UnusedComponent{}
res := def.RunRule(nodes, ctx)

res := setupUnusedComponentsTestContext(t, yml)
assert.Len(t, res, 4)
}

func TestUnusedComponent_RunRule_Success_Fail_Four_Undefined(t *testing.T) {

yml := `paths:
yml := `
openapi: 3.0.0
paths:
/naughty/{puppy}:
parameters:
- $ref: '#/components/parameters/Chewy'
Expand Down Expand Up @@ -223,28 +168,15 @@ components:
in: query
name: chewy`

path := "$"

var rootNode yaml.Node
mErr := yaml.Unmarshal([]byte(yml), &rootNode)
assert.NoError(t, mErr)

nodes, _ := utils.FindNodes([]byte(yml), path)

rule := buildOpenApiTestRuleAction(path, "unused_component", "", nil)
ctx := buildOpenApiTestContext(model.CastToRuleAction(rule.Then), nil)
config := index.CreateOpenAPIIndexConfig()
ctx.Index = index.NewSpecIndexWithConfig(&rootNode, config)

def := UnusedComponent{}
res := def.RunRule(nodes, ctx)

res := setupUnusedComponentsTestContext(t, yml)
assert.Len(t, res, 4)
}

func TestUnusedComponent_RunRule_Success_PolymorphicCheck(t *testing.T) {

yml := `paths:
yml := `
openapi: 3.0.0
paths:
/naughty/{puppy}:
get:
responses:
Expand Down Expand Up @@ -281,28 +213,15 @@ components:
type: string
description: bunny`

path := "$"

var rootNode yaml.Node
mErr := yaml.Unmarshal([]byte(yml), &rootNode)
assert.NoError(t, mErr)

nodes, _ := utils.FindNodes([]byte(yml), path)

rule := buildOpenApiTestRuleAction(path, "unused_component", "", nil)
ctx := buildOpenApiTestContext(model.CastToRuleAction(rule.Then), nil)
config := index.CreateOpenAPIIndexConfig()
ctx.Index = index.NewSpecIndexWithConfig(&rootNode, config)

def := UnusedComponent{}
res := def.RunRule(nodes, ctx)

res := setupUnusedComponentsTestContext(t, yml)
assert.Len(t, res, 0)
}

func TestUnusedComponent_RunRule_Success_PolymorphicCheckAllOf(t *testing.T) {

yml := `paths:
yml := `
openapi: 3.0.0
paths:
"/naughty/{puppy}":
get:
responses:
Expand Down Expand Up @@ -340,21 +259,39 @@ components:
category:
type: String`

res := setupUnusedComponentsTestContext(t, yml)
assert.Len(t, res, 0)
}

func setupUnusedComponentsTestContext(t *testing.T, yml string) []model.RuleFunctionResult {
path := "$"

var rootNode yaml.Node
mErr := yaml.Unmarshal([]byte(yml), &rootNode)
assert.NoError(t, mErr)

document, err := libopenapi.NewDocument([]byte(yml))
if err != nil {
panic(fmt.Sprintf("cannot create new document: %e", err))
}

if strings.Contains(yml, "swagger") {
_, _ = document.BuildV2Model()
} else {
_, _ = document.BuildV3Model()
}

nodes, _ := utils.FindNodes([]byte(yml), path)

rule := buildOpenApiTestRuleAction(path, "unused_component", "", nil)
ctx := buildOpenApiTestContext(model.CastToRuleAction(rule.Then), nil)
config := index.CreateOpenAPIIndexConfig()
ctx.Index = index.NewSpecIndexWithConfig(&rootNode, config)
info, _ := datamodel.ExtractSpecInfo([]byte(yml))
ctx.SpecInfo = info
ctx.Document = document

def := UnusedComponent{}
res := def.RunRule(nodes, ctx)

assert.Len(t, res, 0)
return def.RunRule(nodes, ctx)
}
5 changes: 4 additions & 1 deletion motor/rule_applicator.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ type ruleContext struct {
// of ApplyRulesToRuleSet to change, without a huge refactor. The ApplyRulesToRuleSet function only returns a single error also.
type RuleSetExecution struct {
RuleSet *rulesets.RuleSet // The RuleSet in which to apply
SpecFileName string // The name of the specification file, used to correctly label location
SpecFileName string // The path of the specification file, used to correctly label location
daveshanley marked this conversation as resolved.
Show resolved Hide resolved
Spec []byte // The raw bytes of the OpenAPI specification.
SpecInfo *datamodel.SpecInfo // Pre-parsed spec-info.
CustomFunctions map[string]model.RuleFunction // custom functions loaded from plugin.
Expand Down Expand Up @@ -109,13 +109,16 @@ func ApplyRulesToRuleSet(execution *RuleSetExecution) *RuleSetExecutionResult {

// create new configurations
indexConfig := index.CreateClosedAPIIndexConfig()
indexConfig.SpecFilePath = execution.SpecFileName
indexConfigUnresolved := index.CreateClosedAPIIndexConfig()
indexConfigUnresolved.SpecFilePath = execution.SpecFileName

// avoid building the index, we don't need it to run yet.
indexConfig.AvoidBuildIndex = true
//indexConfig.AvoidCircularReferenceCheck = true

docConfig := datamodel.NewDocumentConfiguration()
docConfig.SpecFilePath = execution.SpecFileName
//docConfig.SkipCircularReferenceCheck = true

if execution.IgnoreCircularArrayRef {
Expand Down
Loading