ronantakizawa/codereview-bench huggingface dataset:
Repository: smicallef/spiderfoot
File: modules/sfp_threatjammer.py (Python)

Code before:
            'description': "Threat Jammer is a service to access high-quality threat intelligence"
            " data from a variety of sources, and integrate it into their applications with the"
            " sole purpose of detecting and blocking malicious activity."
        }
    }

    opts = {
        'api_key': '',
        "risk_score_min": 35,
        'checkaffiliates': True,
    }

    optdescs = {
        'api_key': "ThreatJammer.com API key.",
        "risk_score_min": "Minimum Threat Jammer risk score.",
        'checkaffiliates': "Apply checks to affiliates?",
    }

    results = None

    def setup(self, sfc, userOpts=dict()):
        self.sf = sfc
        self.results = self.tempStorage()

        for opt in list(userOpts.keys()):
            self.opts[opt] = userOpts[opt]

    def watchedEvents(self):
        return [
            "IP_ADDRESS",
            "IPV6_ADDRESS",
            "AFFILIATE_IPADDR",
            "AFFILIATE_IPV6_ADDRESS",
        ]

    def producedEvents(self):
        return [
            "BLACKLISTED_IPADDR",
            "BLACKLISTED_AFFILIATE_IPADDR",
            "MALICIOUS_IPADDR",
            "MALICIOUS_AFFILIATE_IPADDR",
        ]

    def queryIp(self, ip):
        """Query API for an IPv4 or IPv6 address.

        Args:
            ip (str): IP address

        Returns:
            str: API response as JSON

Review comment (suggestion):
```suggestion
        self.results = self.tempStorage()
        errorState = False
```

Code after:
            'description': "Threat Jammer is a service to access high-quality threat intelligence"
            " data from a variety of sources, and integrate it into their applications with the"
            " sole purpose of detecting and blocking malicious activity."
        }
    }

    opts = {
        'api_key': "",
        'api_hostname': "dublin.api.threatjammer.com",
        'risk_score_min': 35,
        'checkaffiliates': True,
    }

    optdescs = {
        'api_key': "API key",
        'api_hostname': "User API hostname",
        'risk_score_min': "Minimum Threat Jammer risk score",
        'checkaffiliates': "Apply checks to affiliates?",
    }

    results = None
    errorState = False

    def setup(self, sfc, userOpts=dict()):
        self.sf = sfc
        self.results = self.tempStorage()
        self.errorState = False

        for opt in list(userOpts.keys()):
            self.opts[opt] = userOpts[opt]

    def watchedEvents(self):
        return [
            "IP_ADDRESS",
            "IPV6_ADDRESS",
            "AFFILIATE_IPADDR",
            "AFFILIATE_IPV6_ADDRESS",
        ]

    def producedEvents(self):
        return [
            "BLACKLISTED_IPADDR",
            "BLACKLISTED_AFFILIATE_IPADDR",
            "MALICIOUS_IPADDR",
            "MALICIOUS_AFFILIATE_IPADDR",
        ]

    def queryIp(self, ip):
        """Query API for an IPv4 or IPv6 address.

        Args:
Repository: smicallef/spiderfoot
File: modules/sfp_threatjammer.py (Python)

Code before:
# -*- coding: utf-8 -*-
# -------------------------------------------------------------------------------
# Name:        sfp_threatjammer
# Purpose:     Check if an IP or netblock is malicious according to ThreatJammer.com.
#
# Author:      diego.parrilla.santamaria@gmail.com
#
# Created:     2022-05-03
# Copyright:   (c) Diego Parrilla 2022
# Licence:     GPL
# -------------------------------------------------------------------------------

import json
import time

from spiderfoot import SpiderFootEvent, SpiderFootPlugin


class sfp_threatjammer(SpiderFootPlugin):

    meta = {
        'name': "Threat Jammer",
        'summary': "Check if an IP address is malicious according to ThreatJammer.com",
        'flags': ["apikey"],
        'useCases': ["Passive", "Investigate"],
        'categories': ["Reputation Systems"],
        'dataSource': {
            'website': "https://threatjammer.com",
            'model': "FREE_AUTH_LIMITED",
            'references': [
                "https://threatjammer.com/docs/what-is-threat-jammer",
                "https://threatjammer.com/docs/how-threat-jammer-works",
                "https://threatjammer.com/docs/introduction-threat-jammer-user-api",
                "https://threatjammer.com/docs/introduction-threat-jammer-report-api",

Review comment (suggestion):
Please change the copyright to `Steve Micallef`. This is required as I need to retain copyright control in the event that I change from GPL to something else.

Code after:
# -*- coding: utf-8 -*-
# -------------------------------------------------------------------------------
# Name:        sfp_threatjammer
# Purpose:     Check if an IP or netblock is malicious according to ThreatJammer.com.
#
# Author:      diego.parrilla.santamaria@gmail.com
#
# Created:     2022-05-03
# Copyright:   (c) Diego Parrilla 2022
# Licence:     MIT
# -------------------------------------------------------------------------------

import json
import time

from spiderfoot import SpiderFootEvent, SpiderFootPlugin


class sfp_threatjammer(SpiderFootPlugin):

    meta = {
        'name': "Threat Jammer",
        'summary': "Check if an IP address is malicious according to ThreatJammer.com",
        'flags': ["apikey"],
        'useCases': ["Passive", "Investigate"],
        'categories': ["Reputation Systems"],
        'dataSource': {
            'website': "https://threatjammer.com",
            'model': "FREE_AUTH_LIMITED",
            'references': [
                "https://threatjammer.com/docs/what-is-threat-jammer",
                "https://threatjammer.com/docs/how-threat-jammer-works",
                "https://threatjammer.com/docs/introduction-threat-jammer-user-api",
                "https://threatjammer.com/docs/introduction-threat-jammer-report-api",
Repository: smicallef/spiderfoot
File: spiderfoot/target.py (Python)

Code before:
import typing

import netaddr


TargetAlias = typing.Dict[typing.Literal["type", "value"], str]


class SpiderFootTarget():
    """SpiderFoot target.

    Attributes:
        validTypes (typing.List[str]): valid event types accepted as a target
        targetType (str): target type
        targetValue (str): target value
        targetAliases (typing.List[TargetAlias]): target aliases
    """

    _validTypes = ["IP_ADDRESS", 'IPV6_ADDRESS', "NETBLOCK_OWNER", "NETBLOCKV6_OWNER", "INTERNET_NAME",
                   "EMAILADDR", "HUMAN_NAME", "BGP_AS_OWNER", 'PHONE_NUMBER', "USERNAME",
                   "BITCOIN_ADDRESS"]
    _targetType: str
    _targetValue: str
    _targetAliases: typing.List[TargetAlias]

    def __init__(self, targetValue: str, typeName: str) -> None:
        """Initialize SpiderFoot target.

        Args:
            targetValue (str): target value
            typeName (str): target type

Review comment (suggestion):
```suggestion
TargetAlias = typing.TypedDict("TargetAlias", {"type": str, "value": str})
```

Code after:
import sys
import typing

import netaddr


if sys.version_info >= (3, 8):  # PEP 589 support (TypedDict)
    TargetAlias = typing.TypedDict("TargetAlias", {"type": str, "value": str})
else:
    TargetAlias = typing.Dict[str, str]


class SpiderFootTarget():
    """SpiderFoot target.

    Attributes:
        validTypes (typing.List[str]): valid event types accepted as a target
        targetType (str): target type
        targetValue (str): target value
        targetAliases (typing.List[TargetAlias]): target aliases
    """

    _validTypes = ["IP_ADDRESS", 'IPV6_ADDRESS', "NETBLOCK_OWNER", "NETBLOCKV6_OWNER", "INTERNET_NAME",
                   "EMAILADDR", "HUMAN_NAME", "BGP_AS_OWNER", 'PHONE_NUMBER', "USERNAME",
                   "BITCOIN_ADDRESS"]
    _targetType: str
    _targetValue: str
    _targetAliases: typing.List[TargetAlias]

    def __init__(self, targetValue: str, typeName: str) -> None:
        """Initialize SpiderFoot target.
Repository: spf13/viper
File: viper_test.go (Go)

Code before:
func TestWrongDirsSearchNotFound(t *testing.T) {

	_, config, cleanup := initDirs(t)
	defer cleanup()

	v := New()
	v.SetConfigName(config)
	v.SetDefault(`key`, `default`)

	v.AddConfigPath(`whattayoutalkingbout`)
	v.AddConfigPath(`thispathaintthere`)

	err := v.ReadInConfig()
	assert.Equal(t, reflect.TypeOf(UnsupportedConfigError("")), reflect.TypeOf(err))

	// Even though config did not load and the error might have
	// been ignored by the client, the default still loads
	assert.Equal(t, `default`, v.GetString(`key`))
}

func TestSub(t *testing.T) {
	v := New()
	v.SetConfigType("yaml")
	v.ReadConfig(bytes.NewBuffer(yamlExample))

	subv := v.Sub("clothing.pants")
	assert.Equal(t, v.Get("clothing.pants.size"), subv.Get("size"))

	subv = v.Sub("clothing.pants.size")
	assert.Equal(t, subv, (*Viper)(nil))
}

var yamlMergeExampleTgt = []byte(`
hello:
    pop: 37890
    world:
    - us
    - uk
    - fr
    - de
`)

var yamlMergeExampleSrc = []byte(`
hello:
    pop: 45000
    universe:
    - mw
    - ad
fu: bar
`)


Review comment (suggestion):
Get("clothing") returns type map[interface{}]interface{}.
Get("clothing.pants") returns type map[string]interface{}.


Code after:
func TestWrongDirsSearchNotFound(t *testing.T) {

	_, config, cleanup := initDirs(t)
	defer cleanup()

	v := New()
	v.SetConfigName(config)
	v.SetDefault(`key`, `default`)

	v.AddConfigPath(`whattayoutalkingbout`)
	v.AddConfigPath(`thispathaintthere`)

	err := v.ReadInConfig()
	assert.Equal(t, reflect.TypeOf(UnsupportedConfigError("")), reflect.TypeOf(err))

	// Even though config did not load and the error might have
	// been ignored by the client, the default still loads
	assert.Equal(t, `default`, v.GetString(`key`))
}

func TestSub(t *testing.T) {
	v := New()
	v.SetConfigType("yaml")
	v.ReadConfig(bytes.NewBuffer(yamlExample))

	subv := v.Sub("clothing")
	assert.Equal(t, v.Get("clothing.pants.size"), subv.Get("pants.size"))

	subv = v.Sub("clothing.pants")
	assert.Equal(t, v.Get("clothing.pants.size"), subv.Get("size"))

	subv = v.Sub("clothing.pants.size")
	assert.Equal(t, subv, (*Viper)(nil))
}

var yamlMergeExampleTgt = []byte(`
hello:
    pop: 37890
    world:
    - us
    - uk
    - fr
    - de
`)

var yamlMergeExampleSrc = []byte(`
hello:
    pop: 45000
    universe:
    - mw
    - ad
Repository: spf13/viper
File: viper.go (Go)

Code before:

	return nil
}

// Given a key, find the value.
// Viper will check in the following order:
// flag, env, config file, key/value store, default.
// Viper will check to see if an alias exists first.
func (v *Viper) find(key string) interface{} {
	var val interface{}
	var exists bool

	// if the requested key is an alias, then return the proper key
	key = v.realKey(key)

	// PFlag Override first
	flag, exists := v.pflags[key]
	if exists && flag.HasChanged() {
		jww.TRACE.Println(key, "found in override (via pflag):", flag.ValueString())
		switch flag.ValueType() {
		case "int", "int8", "int16", "int32", "int64":
			return cast.ToInt(flag.ValueString())
		case "bool":
			return cast.ToBool(flag.ValueString())
		default:
			return flag.ValueString()
		}
	}

	val, exists = v.override[key]
	if exists {
		jww.TRACE.Println(key, "found in override:", val)
		return val
	}

	if v.automaticEnvApplied {
		// even if it hasn't been registered, if automaticEnv is used,
		// check any Get request
		if val = v.getEnv(v.mergeWithEnvPrefix(key)); val != "" {
			jww.TRACE.Println(key, "found in environment with val:", val)
			return val
		}
	}

	envkey, exists := v.env[key]
	if exists {
		jww.TRACE.Println(key, "registered as env var", envkey)
		if val = v.getEnv(envkey); val != "" {
			jww.TRACE.Println(envkey, "found in environment with val:", val)
			return val
		}

Review comment (bug):
This wrongly trims the "[" and "]" in the flags. 

Fix it by:

```
s := flag.ValueString()
return s[1:len(s)-1]
```


Code after:

	return nil
}

// Given a key, find the value.
// Viper will check in the following order:
// flag, env, config file, key/value store, default.
// Viper will check to see if an alias exists first.
func (v *Viper) find(key string) interface{} {
	var val interface{}
	var exists bool

	// if the requested key is an alias, then return the proper key
	key = v.realKey(key)

	// PFlag Override first
	flag, exists := v.pflags[key]
	if exists && flag.HasChanged() {
		jww.TRACE.Printf("%q found in pflag override (%s): %s", key, flag.ValueType(), flag.ValueString())
		switch flag.ValueType() {
		case "int", "int8", "int16", "int32", "int64":
			return cast.ToInt(flag.ValueString())
		case "bool":
			return cast.ToBool(flag.ValueString())
		case "stringSlice":
			s := strings.TrimPrefix(flag.ValueString(), "[")
			return strings.TrimSuffix(s, "]")
		default:
			return flag.ValueString()
		}
	}

	val, exists = v.override[key]
	if exists {
		jww.TRACE.Printf("%q found in override: %s", key, val)
		return val
	}

	if v.automaticEnvApplied {
		// even if it hasn't been registered, if automaticEnv is used,
		// check any Get request
		if val = v.getEnv(v.mergeWithEnvPrefix(key)); val != "" {
			jww.TRACE.Printf("%q found in environment: %s", key, val)
			return val
		}
	}

	envkey, exists := v.env[key]
	if exists {
		jww.TRACE.Printf("%q registered as env var %q", key, envkey)
		if val = v.getEnv(envkey); val != "" {
Repository: spf13/viper
File: viper_test.go (Go)

Code before:
	v.SetFs(fs)
	v.SetConfigName("c")
	v.SetConfigType("yaml")
	err := v.ReadConfig(bytes.NewBuffer(yamlExample))
	if err != nil {
		t.Fatal(err)
	}
	if err := v.WriteConfigAs("c.yaml"); err != nil {
		t.Fatal(err)
	}
	read, err := afero.ReadFile(fs, "c.yaml")
	if err != nil {
		t.Fatal(err)
	}
	assert.Equal(t, yamlWriteExpected, read)
}

func TestSafeWriteConfig(t *testing.T) {
	v := New()
	fs := afero.NewMemMapFs()
	v.SetFs(fs)
	v.AddConfigPath("/test")
	v.SetConfigName("c")
	v.SetConfigType("yaml")
	err := v.ReadConfig(bytes.NewBuffer(yamlExample))
	if err != nil {
		t.Fatal(err)
	}
	if err = v.SafeWriteConfig(); err != nil {
		t.Fatal(err)
	}
	read, err := afero.ReadFile(fs, "/test/c.yaml")
	if err != nil {
		t.Fatal(err)
	}
	assert.Equal(t, yamlWriteExpected, read)
}

func TestSafeWriteConfigWithMissingConfigPath(t *testing.T) {
	v := New()
	fs := afero.NewMemMapFs()
	v.SetFs(fs)
	v.SetConfigName("c")
	v.SetConfigType("yaml")
	err := v.SafeWriteConfig()
	if err == nil {
		t.Fatal("Expected exception")
	}
	_, ok := err.(MissingConfigurationError)
	assert.True(t, ok, "Expected MissingConfigurationError")
}

Review comment (suggestion):
We tend to use `require.NoError` here (same for bellow).

Code after:
	v.SetFs(fs)
	v.SetConfigName("c")
	v.SetConfigType("yaml")
	err := v.ReadConfig(bytes.NewBuffer(yamlExample))
	if err != nil {
		t.Fatal(err)
	}
	if err := v.WriteConfigAs("c.yaml"); err != nil {
		t.Fatal(err)
	}
	read, err := afero.ReadFile(fs, "c.yaml")
	if err != nil {
		t.Fatal(err)
	}
	assert.Equal(t, yamlWriteExpected, read)
}

func TestSafeWriteConfig(t *testing.T) {
	v := New()
	fs := afero.NewMemMapFs()
	v.SetFs(fs)
	v.AddConfigPath("/test")
	v.SetConfigName("c")
	v.SetConfigType("yaml")
	require.NoError(t, v.ReadConfig(bytes.NewBuffer(yamlExample)))
	require.NoError(t, v.SafeWriteConfig())
	read, err := afero.ReadFile(fs, "/test/c.yaml")
	require.NoError(t, err)
	assert.Equal(t, yamlWriteExpected, read)
}

func TestSafeWriteConfigWithMissingConfigPath(t *testing.T) {
	v := New()
	fs := afero.NewMemMapFs()
	v.SetFs(fs)
	v.SetConfigName("c")
	v.SetConfigType("yaml")
	require.EqualError(t, v.SafeWriteConfig(), "Missing configuration for 'configPath'")
}

func TestSafeWriteConfigWithExistingFile(t *testing.T) {
	v := New()
	fs := afero.NewMemMapFs()
	fs.Create("/test/c.yaml")
	v.SetFs(fs)
	v.AddConfigPath("/test")
	v.SetConfigName("c")
	v.SetConfigType("yaml")
	err := v.SafeWriteConfig()
	require.Error(t, err)
	_, ok := err.(ConfigFileAlreadyExistsError)
Repository: spf13/viper
File: viper_test.go (Go)

Code before:
var propertiesWriteExpected = []byte(`p_id = 0001
p_type = donut
p_name = Cake
p_ppu = 0.55
p_batters.batter.type = Regular
`)

var yamlWriteExpected = []byte(`age: 35
beard: true
clothing:
  jacket: leather
  pants:
    size: large
  trousers: denim
eyes: brown
hacker: true
hobbies:
- skateboarding
- snowboarding
- go
name: steve
`)

func TestWriteConfig(t *testing.T) {
	fs := afero.NewMemMapFs()
	testCases := map[string]struct {
		configName      string
		configType      string
		fileName        string
		input           []byte
		expectedContent []byte
	}{
		"hcl with file extension": {
			configName:      "c",
			configType:      "hcl",
			fileName:        "c.hcl",
			input:           hclExample,
			expectedContent: hclWriteExpected,
		},
		"hcl without file extension": {
			configName:      "c",
			configType:      "hcl",
			fileName:        "c",
			input:           hclExample,
			expectedContent: hclWriteExpected,
		},
		"json with file extension": {
			configName:      "c",
			configType:      "json",
			fileName:        "c.json",
			input:           jsonExample,

Review comment (refactor):
Nice refactor!

Currently, the config type is always inferred from the file name, not the original config type. So if the config type was yaml, but I provided a json file name, the output content was in JSON.

Can you please add a test case that verifies this is still the case?

Because (unfortunately) as far as I can tell, it is not.

Code after:
var propertiesWriteExpected = []byte(`p_id = 0001
p_type = donut
p_name = Cake
p_ppu = 0.55
p_batters.batter.type = Regular
`)

var yamlWriteExpected = []byte(`age: 35
beard: true
clothing:
  jacket: leather
  pants:
    size: large
  trousers: denim
eyes: brown
hacker: true
hobbies:
- skateboarding
- snowboarding
- go
name: steve
`)

func TestWriteConfig(t *testing.T) {
	fs := afero.NewMemMapFs()
	testCases := map[string]struct {
		configName      string
		inConfigType    string
		outConfigType   string
		fileName        string
		input           []byte
		expectedContent []byte
	}{
		"hcl with file extension": {
			configName:      "c",
			inConfigType:    "hcl",
			outConfigType:   "hcl",
			fileName:        "c.hcl",
			input:           hclExample,
			expectedContent: hclWriteExpected,
		},
		"hcl without file extension": {
			configName:      "c",
			inConfigType:    "hcl",
			outConfigType:   "hcl",
			fileName:        "c",
			input:           hclExample,
			expectedContent: hclWriteExpected,
		},
		"hcl with file extension and mismatch type": {
			configName:      "c",
Repository: spf13/viper
File: viper.go (Go)

Code before:

	return ""
}

func (v *Viper) getConfigFile() (string, error) {
	if v.configFile == "" {
		cf, err := v.findConfigFile()
		if err != nil {
			return "", err
		}
		v.configFile = cf
	}
	return v.configFile, nil
}

func (v *Viper) searchInPath(in string) (filename string) {
	jww.DEBUG.Println("Searching for config in ", in)
	for _, ext := range SupportedExts {
		jww.DEBUG.Println("Checking for", filepath.Join(in, v.configName+"."+ext))
		if b, _ := exists(v.fs, filepath.Join(in, v.configName+"."+ext)); b {
			jww.DEBUG.Println("Found: ", filepath.Join(in, v.configName+"."+ext))
			return filepath.Join(in, v.configName+"."+ext)
		}
	}

	if b, _ := exists(v.fs, filepath.Join(in, v.configName)); b && v.configType != "" {
		return filepath.Join(in, v.configName)
	}

	return ""
}

// Search all configPaths for any config file.
// Returns the first path that exists (and is a config file).
func (v *Viper) findConfigFile() (string, error) {
	jww.INFO.Println("Searching for config in ", v.configPaths)

	for _, cp := range v.configPaths {
		file := v.searchInPath(cp)
		if file != "" {
			return file, nil
		}
	}
	return "", ConfigFileNotFoundError{v.configName, fmt.Sprintf("%s", v.configPaths)}
}

// Debug prints all configuration registries for debugging
// purposes.
func Debug() { v.Debug() }
func (v *Viper) Debug() {
	fmt.Printf("Aliases:\n%#v\n", v.aliases)

Review comment (performance):
It's a minor point, performance-wise I'd do the simple string check first. Not sure it matters. Otherwise LGTM, much appreciated.

Code after:

	return ""
}

func (v *Viper) getConfigFile() (string, error) {
	if v.configFile == "" {
		cf, err := v.findConfigFile()
		if err != nil {
			return "", err
		}
		v.configFile = cf
	}
	return v.configFile, nil
}

func (v *Viper) searchInPath(in string) (filename string) {
	jww.DEBUG.Println("Searching for config in ", in)
	for _, ext := range SupportedExts {
		jww.DEBUG.Println("Checking for", filepath.Join(in, v.configName+"."+ext))
		if b, _ := exists(v.fs, filepath.Join(in, v.configName+"."+ext)); b {
			jww.DEBUG.Println("Found: ", filepath.Join(in, v.configName+"."+ext))
			return filepath.Join(in, v.configName+"."+ext)
		}
	}

	if v.configType != "" {
		if b, _ := exists(v.fs, filepath.Join(in, v.configName)); b {
			return filepath.Join(in, v.configName)
		}
	}

	return ""
}

// Search all configPaths for any config file.
// Returns the first path that exists (and is a config file).
func (v *Viper) findConfigFile() (string, error) {
	jww.INFO.Println("Searching for config in ", v.configPaths)

	for _, cp := range v.configPaths {
		file := v.searchInPath(cp)
		if file != "" {
			return file, nil
		}
	}
	return "", ConfigFileNotFoundError{v.configName, fmt.Sprintf("%s", v.configPaths)}
}

// Debug prints all configuration registries for debugging
// purposes.
func Debug() { v.Debug() }