Skip to content

ci: Enforce Static Code Analyzer #155

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

Merged
merged 7 commits into from
Jan 24, 2019
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
17 changes: 11 additions & 6 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,29 +6,34 @@ php:
- '7.1'
- '7.2'
- '7.3'
before_install:
- composer install --dev
install: "composer install"
addons:
srcclr: true
script:
- mkdir -p build/logs
- ./vendor/bin/phpunit --coverage-clover build/logs/clover.xml
after_script:
- vendor/bin/php-coveralls -v
after_script: "vendor/bin/php-coveralls -v"

# Integration tests need to run first to reset the PR build status to pending
# Linting and integration tests need to run first to reset the PR build status to pending
stages:
- 'Linting'
- 'Integration tests'
- 'Test'

jobs:
include:
- stage: 'Linting'
language: php
php: '7.0'
install: 'composer require "squizlabs/php_codesniffer=*"'
script: 'composer lint'
after_script: skip
after_success: travis_terminate 0
- stage: 'Integration tests'
merge_mode: replace
env: SDK=php
cache: false
language: python
before_install: skip
install:
- "pip install awscli"
before_script:
Expand Down
5 changes: 5 additions & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,11 @@ We welcome contributions and feedback! All contributors must sign our [Contribut

* **All code must have test coverage.** We use PHPUnit. Changes in functionality should have accompanying unit tests. Bug fixes should have accompanying regression tests.
* Tests are located in `tests` with one file per class.
* Lint your code with PHP CodeSniffer before submitting.

## Style
We enforce [PSR-2](https://www.php-fig.org/psr/psr-2/) rules with some minor [deviations](phpcs.xml). Run linter by executing `composer lint` and autocorrect lint errors by executing `composer beautify`.


## License

Expand Down
8 changes: 7 additions & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,11 @@
"email": "[email protected]"
}
],
"scripts": {
"test": "phpunit",
"lint": "phpcs",
"beautify": "phpcbf"
},
"require": {
"php": ">=5.5",
"justinrainbow/json-schema": "^1.6 || ^2.0 || ^4.0 || ^5.0",
Expand All @@ -19,7 +24,8 @@
},
"require-dev": {
"phpunit/phpunit": "^4.8|^5.0",
"php-coveralls/php-coveralls": "v2.0.0"
"php-coveralls/php-coveralls": "v2.0.0",
"squizlabs/php_codesniffer": "3.*"
},
"autoload": {
"psr-4": {
Expand Down
23 changes: 23 additions & 0 deletions phpcs.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<?xml version="1.0"?>
<ruleset name="OptimizelyPSR2">
<!-- description for custom ruleset -->
<description>A custom coding standard for Optimizely PHP-SDK based on PSR2</description>

<!-- If no files or directories are specified on commandline, these files will be sniffed -->
<file>./src</file>
<file>./tests</file>

<!-- Exclude patterns for files to be excluded from sniffing -->
<!-- <exclude-pattern>./tests/EventTests/*</exclude-pattern> -->

<!-- Embed command line arguments in config file. -->
<arg name="tab-width" value="4"/>

<!-- To exclude any rule sniff, get sniff name by running phpcs with -s switch -->
<rule ref="PSR2">
<exclude name="Generic.Files.LineLength.TooLong"/>
<exclude name="PSR2.Classes.PropertyDeclaration.Underscore"/>
<exclude name="PSR1.Classes.ClassDeclaration.MultipleClasses"/>
<exclude name="PSR1.Files.SideEffects.FoundWithSymbols"/>
</rule >
</ruleset>
6 changes: 3 additions & 3 deletions src/Optimizely/Bucketer.php
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<?php
/**
* Copyright 2016-2018, Optimizely
* Copyright 2016-2019, Optimizely
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -90,9 +90,9 @@ protected function generateBucketValue($bucketingKey)

/* murmurhash3_int returns both positive and negative integers for PHP x86 versions
it returns negative integers when it tries to create 2^32 integers while PHP doesn't support
unsigned integers and can store integers only upto 2^31.
unsigned integers and can store integers only upto 2^31.
Observing generated hashcodes and their corresponding bucket values after normalization
indicates that a negative bucket number on x86 is exactly 10,000 less than it's
indicates that a negative bucket number on x86 is exactly 10,000 less than it's
corresponding bucket number on x64. Hence we can safely add 10,000 to a negative number to
make it consistent across both of the PHP variants. */

Expand Down
8 changes: 4 additions & 4 deletions src/Optimizely/DecisionService/DecisionService.php
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<?php
/**
* Copyright 2017-2018, Optimizely Inc and Contributors
* Copyright 2017-2019, Optimizely Inc and Contributors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -93,8 +93,8 @@ protected function getBucketingId($userId, $userAttributes)
{
$bucketingIdKey = ControlAttributes::BUCKETING_ID;

if (isset($userAttributes[$bucketingIdKey])){
if (is_string($userAttributes[$bucketingIdKey])){
if (isset($userAttributes[$bucketingIdKey])) {
if (is_string($userAttributes[$bucketingIdKey])) {
return $userAttributes[$bucketingIdKey];
}
$this->_logger->log(Logger::WARNING, 'Bucketing ID attribute is not a string. Defaulted to user ID.');
Expand Down Expand Up @@ -308,7 +308,7 @@ public function getVariationForFeatureRollout(FeatureFlag $featureFlag, $userId,
break;
}
// Evaluate Everyone Else Rule / Last Rule now
$experiment = $rolloutRules[sizeof($rolloutRules)-1];
$experiment = $rolloutRules[sizeof($rolloutRules)-1];

// Evaluate if user meets the audience condition of Everyone Else Rule / Last Rule now
if (!Validator::isUserInExperiment($this->_projectConfig, $experiment, $userAttributes)) {
Expand Down
4 changes: 2 additions & 2 deletions src/Optimizely/Entity/Experiment.php
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<?php
/**
* Copyright 2016, 2018, Optimizely
* Copyright 2016, 2018-2019, Optimizely
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -78,7 +78,7 @@ class Experiment

/**
* @var array/string Single audience ID the experiment is attached to or
* hierarchical conditions array of audience IDs related by AND/OR/NOT operators.
* hierarchical conditions array of audience IDs related by AND/OR/NOT operators.
*/
private $_audienceConditions;

Expand Down
4 changes: 2 additions & 2 deletions src/Optimizely/Entity/FeatureVariable.php
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<?php
/**
* Copyright 2017, Optimizely
* Copyright 2017, 2019, Optimizely
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -61,7 +61,7 @@ class FeatureVariable
private $_defaultValue;


public function __construct($id =null, $key = null, $type = null, $defaultValue = null)
public function __construct($id = null, $key = null, $type = null, $defaultValue = null)
{
$this->_id = $id;
$this->_key = $key;
Expand Down
4 changes: 2 additions & 2 deletions src/Optimizely/Entity/Variation.php
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<?php
/**
* Copyright 2016-2018, Optimizely
* Copyright 2016-2019, Optimizely
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -115,7 +115,7 @@ public function getVariables()
}

/**
* @param string Variable ID
* @param string Variable ID
*
* @return VariableUsage Variable usage instance corresponding to given variable ID
*/
Expand Down
4 changes: 2 additions & 2 deletions src/Optimizely/Event/Builder/EventBuilder.php
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ private function getCommonParams($config, $userId, $attributes)
ENRICH_DECISIONS => true
];

if(!is_null($attributes)) {
if (!is_null($attributes)) {
foreach ($attributes as $attributeKey => $attributeValue) {
// Omit attributes that are not supported by the log endpoint.
if (Validator::isAttributeValid($attributeKey, $attributeValue)) {
Expand Down Expand Up @@ -199,7 +199,7 @@ private function getConversionParams($eventEntity, $eventTags)
$eventDict[EventTagUtils::NUMERIC_EVENT_METRIC_NAME] = $eventValue;
}

if(count($eventTags) > 0) {
if (count($eventTags) > 0) {
$eventDict['tags'] = $eventTags;
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/Optimizely/Event/Builder/Params.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
define('DECISIONS', 'decisions');
define('ENRICH_DECISIONS', 'enrich_decisions');
define('ENTITY_ID', 'entity_id');
define('EVENTS', 'events');;
define('EVENTS', 'events');
define('EXPERIMENT_ID', 'experiment_id');
define('KEY', 'key');
define('PROJECT_ID', 'project_id');
Expand Down
6 changes: 3 additions & 3 deletions src/Optimizely/Logger/DefaultLogger.php
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<?php
/**
* Copyright 2016,2018 Optimizely
* Copyright 2016, 2018-2019 Optimizely
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -35,8 +35,8 @@ class DefaultLogger implements LoggerInterface
/**
* DefaultLogger constructor.
*
* @param int $minLevel Minimum level of messages to be logged.
* @param string $stream The PHP stream to log output.
* @param int $minLevel Minimum level of messages to be logged.
* @param string $stream The PHP stream to log output.
*/
public function __construct($minLevel = Logger::INFO, $stream = "stdout")
{
Expand Down
30 changes: 16 additions & 14 deletions src/Optimizely/Notification/NotificationCenter.php
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<?php
/**
* Copyright 2017-2018, Optimizely Inc and Contributors
* Copyright 2017-2019, Optimizely Inc and Contributors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -58,7 +58,7 @@ public function getNotifications()
/**
* Adds a notification callback for a notification type to the notification center
*
* @param string $notification_type One of the constants defined in NotificationType
* @param string $notification_type One of the constants defined in NotificationType
* @param callable $notification_callback A valid PHP callback
*
* @return null Given invalid notification type/callback
Expand Down Expand Up @@ -122,12 +122,13 @@ public function removeNotificationListener($notification_id)
*
* @deprecated Use 'clearNotificationListeners' instead.
*/
public function clearNotifications($notification_type){
$this->_logger->log(
Logger::WARNING,
"'clearNotifications' is deprecated. Call 'clearNotificationListeners' instead."
);
$this->clearNotificationListeners($notification_type);
public function clearNotifications($notification_type)
{
$this->_logger->log(
Logger::WARNING,
"'clearNotifications' is deprecated. Call 'clearNotificationListeners' instead."
);
$this->clearNotificationListeners($notification_type);
}

/**
Expand All @@ -152,12 +153,13 @@ public function clearNotificationListeners($notification_type)
*
* @deprecated Use 'clearAllNotificationListeners' instead.
*/
public function cleanAllNotifications(){
$this->_logger->log(
Logger::WARNING,
"'cleanAllNotifications' is deprecated. Call 'clearAllNotificationListeners' instead."
);
$this->clearAllNotificationListeners();
public function cleanAllNotifications()
{
$this->_logger->log(
Logger::WARNING,
"'cleanAllNotifications' is deprecated. Call 'clearAllNotificationListeners' instead."
);
$this->clearAllNotificationListeners();
}

/**
Expand Down
12 changes: 6 additions & 6 deletions src/Optimizely/Optimizely.php
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,7 @@ public function activate($experimentKey, $userId, $attributes = null)
self::EXPERIMENT_KEY =>$experimentKey,
self::USER_ID => $userId
]
)
)
) {
return null;
}
Expand Down Expand Up @@ -301,7 +301,7 @@ public function track($eventKey, $userId, $attributes = null, $eventTags = null)
self::EVENT_KEY =>$eventKey,
self::USER_ID => $userId
]
)
)
) {
return null;
}
Expand Down Expand Up @@ -389,7 +389,7 @@ public function getVariation($experimentKey, $userId, $attributes = null)
self::EXPERIMENT_KEY =>$experimentKey,
self::USER_ID => $userId
]
)
)
) {
return null;
}
Expand Down Expand Up @@ -484,7 +484,7 @@ public function isFeatureEnabled($featureFlagKey, $userId, $attributes = null)
self::FEATURE_FLAG_KEY =>$featureFlagKey,
self::USER_ID => $userId
]
)
)
) {
return false;
}
Expand Down Expand Up @@ -539,7 +539,7 @@ public function getEnabledFeatures($userId, $attributes = null)
[
self::USER_ID => $userId
]
)
)
) {
return $enabledFeatureKeys;
}
Expand Down Expand Up @@ -584,7 +584,7 @@ public function getFeatureVariableValueForType(
self::VARIABLE_KEY => $variableKey,
self::USER_ID => $userId
]
)
)
) {
return null;
}
Expand Down
10 changes: 4 additions & 6 deletions src/Optimizely/ProjectConfig.php
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ public function __construct($datafile, $logger, $errorHandler)
$this->_logger = $logger;
$this->_errorHandler = $errorHandler;
$this->_version = $config['version'];
if(!in_array($this->_version, $supportedVersions)){
if (!in_array($this->_version, $supportedVersions)) {
throw new InvalidDatafileVersionException(
"This version of the PHP SDK does not support the given datafile version: {$this->_version}."
);
Expand Down Expand Up @@ -256,7 +256,7 @@ public function __construct($datafile, $logger, $errorHandler)
}

// Conditions in typedAudiences are not expected to be string-encoded so they don't need
// to be decoded unlike audiences.
// to be decoded unlike audiences.
foreach (array_values($typedAudienceIdMap) as $typedAudience) {
$typedAudience->setConditionsList($typedAudience->getConditions());
}
Expand Down Expand Up @@ -402,7 +402,7 @@ public function getExperimentFromId($experimentId)
}

/**
* @param String $featureKey Key of the feature flag
* @param String $featureKey Key of the feature flag
*
* @return FeatureFlag Entity corresponding to the key.
*/
Expand All @@ -418,7 +418,7 @@ public function getFeatureFlagFromKey($featureKey)
}

/**
* @param String $rolloutId
* @param String $rolloutId
*
* @return Rollout
*/
Expand Down Expand Up @@ -577,8 +577,6 @@ public function getFeatureVariableFromKey($featureFlagKey, $variableKey)

$this->_logger->log(
Logger::ERROR,


sprintf(
'No variable key "%s" defined in datafile for feature flag "%s".',
$variableKey,
Expand Down
Loading