From f2d381790f41cbdaac824ad50aa1e6b58d55004e Mon Sep 17 00:00:00 2001 From: Chris Smowton Date: Thu, 18 Feb 2021 08:57:04 +0000 Subject: [PATCH] Initial commit --- checkReturnValue.go | 118 ++++++++++++++++++++++++++++++++++++ conditionalPolarities.go | 69 +++++++++++++++++++++ logicalOperators.go | 125 +++++++++++++++++++++++++++++++++++++++ moreWaysToReturn.go | 78 ++++++++++++++++++++++++ utils.go | 19 ++++++ wrapperFunctions.go | 105 ++++++++++++++++++++++++++++++++ 6 files changed, 514 insertions(+) create mode 100644 checkReturnValue.go create mode 100644 conditionalPolarities.go create mode 100644 logicalOperators.go create mode 100644 moreWaysToReturn.go create mode 100644 utils.go create mode 100644 wrapperFunctions.go diff --git a/checkReturnValue.go b/checkReturnValue.go new file mode 100644 index 0000000..0f6452c --- /dev/null +++ b/checkReturnValue.go @@ -0,0 +1,118 @@ +package minioExtensions + +import "errors" + +func thenBranchGoodError() error { + + if errorSource() != ErrNone { + // Good: returning an error + return errors.New("failed") + } + doSomething() + return nil + +} + +func thenBranchGoodWithElseError() error { + + if errorSource() != ErrNone { + // Good: an error means we return an error + return errors.New("failed") + } else { + doSomething() + } + doSomething() + return nil + +} + +func thenBranchBadError() error { + + if errorSource() != ErrNone { + // Bad: despite an error, we return nil + return nil + } + doSomething() + return nil + +} + +func thenBranchBadWithElseError() error { + + if errorSource() != ErrNone { + // Bad: despite an error, we return nil + return nil + } else { + doSomething() + } + doSomething() + return nil + +} + +func elseBranchGoodError() error { + + if errorSource() == ErrNone { + doSomething() + } else { + // Good: returning an error + return errors.New("failed") + } + doSomething() + return nil +} + +func elseBranchBadError() error { + + if errorSource() == ErrNone { + doSomething() + } else { + // Bad: despite an error, we return nil + return nil + } + doSomething() + return nil + +} + +func multiReturnBad() (string, error) { + + if errorSource() != ErrNone { + // Bad: despite an error, we return a nil error + return "", nil + } + doSomething() + return "Result", nil + +} + +func getNil() error { + return nil +} + +func getError(s string) error { + return errors.New(s) +} + +func thenBranchGoodInterproceduralError() error { + + if errorSource() != ErrNone { + // Good: returning an error + return getError("failed") + } + doSomething() + return getNil() + +} + +func thenBranchBadInterproceduralError() error { + + if errorSource() != ErrNone { + // Good: returning nil despite an error + return getNil() + } + doSomething() + return getNil() + +} + diff --git a/conditionalPolarities.go b/conditionalPolarities.go new file mode 100644 index 0000000..e53546e --- /dev/null +++ b/conditionalPolarities.go @@ -0,0 +1,69 @@ +package minioExtensions + +func thenBranchGood() { + + if errorSource() != ErrNone { + // Good: an error means we return early + return + } + doSomething() + +} + +func thenBranchGoodWithElse() { + + if errorSource() != ErrNone { + // Good: an error means we return early + return + } else { + doSomething() + } + doSomething() + +} + +func thenBranchBad() { + + if errorSource() != ErrNone { + // Bad: despite an error, we carry on to execute doSomething() + insteadOfReturn() + } + doSomething() + +} + +func thenBranchBadWithElse() { + + if errorSource() != ErrNone { + // Bad: despite an error, we carry on to execute doSomething() + insteadOfReturn() + } else { + doSomething() + } + doSomething() + +} + +func elseBranchGood() { + + if errorSource() == ErrNone { + doSomething() + } else { + // Good: an error means we return early + return + } + doSomething() + +} + +func elseBranchBad() { + + if errorSource() == ErrNone { + doSomething() + } else { + // Bad: despite an error, we carry on to execute doSomething() + insteadOfReturn() + } + doSomething() + +} diff --git a/logicalOperators.go b/logicalOperators.go new file mode 100644 index 0000000..3d5835d --- /dev/null +++ b/logicalOperators.go @@ -0,0 +1,125 @@ +package minioExtensions + +func negatedThenBranchGood() { + + if !(errorSource() == ErrNone) { + // Good: an error means we return early + return + } + doSomething() + +} + +func negatedThenBranchGoodWithElse() { + + if !(errorSource() == ErrNone) { + // Good: an error means we return early + return + } else { + doSomething() + } + doSomething() + +} + +func negatedElseBranchGood() { + + if !(errorSource() != ErrNone) { + doSomething() + } else { + // Good: an error means we return early + return + } + doSomething() + +} + +func negatedElseBranchBad() { + + if !(errorSource() != ErrNone) { + doSomething() + } else { + // Bad: despite an error, we carry on to execute doSomething() + insteadOfReturn() + } + doSomething() + +} + +func someOtherCondition() bool { + return true +} + +func logicalAndThenBranchGood() { + + if errorSource() != ErrNone && someOtherCondition() { + // Good: in the then-block, where we're certain there has been an error, + // we return early. In the other branch an error is possible but not certain. + return + } + doSomething() + +} + +func logicalAndThenBranchBad() { + + if errorSource() != ErrNone && someOtherCondition() { + // Bad: in the then-block, where we're certain there has been an error, + // we do not return early. + insteadOfReturn() + } + doSomething() + +} + +func logicalAndElseBranchUncertain() { + + if errorSource() == ErrNone && someOtherCondition() { + doSomething() + } else { + // Uncertain: an error is not a precondition for entering either branch. + // We should be conservative and NOT flag this function as a problem. + insteadOfReturn() + } + doSomething() + +} + +func logicalOrElseBranchGood() { + + if errorSource() == ErrNone || someOtherCondition() { + doSomething() + } else { + // Good: in the else-block, where we're certain there has been an error, + // we return early. In the other branch an error is possible but not certain. + return + } + doSomething() + +} + +func logicalOrElseBranchBad() { + + if errorSource() == ErrNone || someOtherCondition() { + doSomething() + } else { + // Bad: in the else-block, where we're certain there has been an error, + // we do not return early. + insteadOfReturn() + } + doSomething() + +} + +func logicalOrThenBranchUncertain() { + + if errorSource() != ErrNone || someOtherCondition() { + // Uncertain: an error is not a precondition for entering either branch. + // We should be conservative and NOT flag this function as a problem. + insteadOfReturn() + } else { + doSomething() + } + doSomething() + +} diff --git a/moreWaysToReturn.go b/moreWaysToReturn.go new file mode 100644 index 0000000..6048333 --- /dev/null +++ b/moreWaysToReturn.go @@ -0,0 +1,78 @@ +package minioExtensions + +const particularErrOne = 1 +const particularErrTwo = 2 + +func subBranchGood() int { + + // Good: while the if-block's logic is complex, it always returns. + err := errorSource() + if err != ErrNone { + if err == particularErrOne { + return -1 + } else if err == particularErrTwo { + return -2 + } else { + return -3 + } + } + doSomething() + return 0 + +} + +func subBranchBad() int { + + // Bad: one of the if-block's branches falls through to execute `doSomething()`. + err := errorSource() + if err != ErrNone { + if err == particularErrOne { + return -1 + } else if err == particularErrTwo { + err = ErrNone + } else { + return -3 + } + } + doSomething() + return 0 + +} + +func switchGood() int { + + // Good: while the if-block's logic is complex, it always returns. + err := errorSource() + if err != ErrNone { + switch err { + case particularErrOne: + return -1 + case particularErrTwo: + return -2 + default: + return -3 + } + } + doSomething() + return 0 + +} + +func switchBad() int { + + // Bad: one of the if-block's branches falls through to execute `doSomething()`. + err := errorSource() + if err != ErrNone { + switch err { + case particularErrOne: + err = ErrNone + case particularErrTwo: + return -2 + default: + return -3 + } + } + doSomething() + return 0 + +} \ No newline at end of file diff --git a/utils.go b/utils.go new file mode 100644 index 0000000..c50a330 --- /dev/null +++ b/utils.go @@ -0,0 +1,19 @@ +package minioExtensions + +func errorSource() int { + // The source of error values that must be checked. + // We should treat this the same as `isReqAuthenticated` in the real `minio`. + return 0 +} + +// The ErrNone value that errorSource() is compared against. +var ErrNone = 0 + +func doSomething() { + // A no-op used as a stand-in for whatever code would normally follow an error check. +} + +// A filler function, used to occupy a then- or else-block that would otherwise be empty, +// where a return would be expected +func insteadOfReturn() { +} diff --git a/wrapperFunctions.go b/wrapperFunctions.go new file mode 100644 index 0000000..8d41d66 --- /dev/null +++ b/wrapperFunctions.go @@ -0,0 +1,105 @@ +package minioExtensions + +func success() bool { + return errorSource() == ErrNone +} + +func failure() bool { + return errorSource() != ErrNone +} + +func succeeded(ret int) bool { + return ret == ErrNone +} + +func failed(ret int) bool { + return ret != ErrNone +} + +func thenBranchGoodSourceWrapper() { + + if failure() { + // Good: an error means we return early + return + } + doSomething() + +} + +func thenBranchBadSourceWrapper() { + + if failure() { + // Bad: despite an error, we carry on to execute doSomething() + insteadOfReturn() + } + doSomething() + +} + +func elseBranchGoodSourceWrapper() { + + if success() { + doSomething() + } else { + // Good: an error means we return early + return + } + doSomething() + +} + +func elseBranchBadSourceWrapper() { + + if success() { + doSomething() + } else { + // Bad: despite an error, we carry on to execute doSomething() + insteadOfReturn() + } + doSomething() + +} + +func thenBranchGoodTestWrapper() { + + if failed(errorSource()) { + // Good: an error means we return early + return + } + doSomething() + +} + +func thenBranchBadTestWrapper() { + + if failed(errorSource()) { + // Bad: despite an error, we carry on to execute doSomething() + insteadOfReturn() + } + doSomething() + +} + +func elseBranchGoodTestWrapper() { + + if succeeded(errorSource()) { + doSomething() + } else { + // Good: an error means we return early + return + } + doSomething() + +} + +func elseBranchBadTestWrapper() { + + if succeeded(errorSource()) { + doSomething() + } else { + // Bad: despite an error, we carry on to execute doSomething() + insteadOfReturn() + } + doSomething() + +}