From 97832288f366dae27584956414242e18dc96bca3 Mon Sep 17 00:00:00 2001 From: Elena Vilchik Date: Fri, 23 Dec 2016 17:15:31 +0100 Subject: [PATCH] SONARJS-67 Rule: Variables should not be declared and then immediately returned or thrown (#429) --- .../src/test/expected/javascript-S1488.json | 540 ++++++++++++++++++ .../sonar/javascript/checks/CheckList.java | 1 + .../ImmediatelyReturnedVariableCheck.java | 110 ++++ .../javascript/rules/javascript/S1488.html | 18 + .../javascript/rules/javascript/S1488.json | 13 + .../rules/javascript/Sonar_way_profile.json | 1 + .../ImmediatelyReturnedVariableCheckTest.java | 32 ++ .../checks/ImmediatelyReturnedVariable.js | 120 ++++ 8 files changed, 835 insertions(+) create mode 100644 its/ruling/src/test/expected/javascript-S1488.json create mode 100644 javascript-checks/src/main/java/org/sonar/javascript/checks/ImmediatelyReturnedVariableCheck.java create mode 100644 javascript-checks/src/main/resources/org/sonar/l10n/javascript/rules/javascript/S1488.html create mode 100644 javascript-checks/src/main/resources/org/sonar/l10n/javascript/rules/javascript/S1488.json create mode 100644 javascript-checks/src/test/java/org/sonar/javascript/checks/ImmediatelyReturnedVariableCheckTest.java create mode 100644 javascript-checks/src/test/resources/checks/ImmediatelyReturnedVariable.js diff --git a/its/ruling/src/test/expected/javascript-S1488.json b/its/ruling/src/test/expected/javascript-S1488.json new file mode 100644 index 00000000000..d4d10eac092 --- /dev/null +++ b/its/ruling/src/test/expected/javascript-S1488.json @@ -0,0 +1,540 @@ +{ +'project:backbone-0.9.2.js':[ +1079, +], +'project:dojo-1.8.1/dijit/_TimePicker.js.uncompressed.js':[ +30, +], +'project:dojo-1.8.1/dijit/_editor/RichText.js.uncompressed.js':[ +48, +], +'project:dojo-1.8.1/dijit/form/RangeBoundTextBox.js.uncompressed.js':[ +11, +], +'project:dojo-1.8.1/dijit/form/_DateTimeTextBox.js.uncompressed.js':[ +19, +], +'project:dojo-1.8.1/dijit/form/_FormSelectWidget.js.uncompressed.js':[ +35, +], +'project:dojo-1.8.1/dijit/tree/dndSource.js.uncompressed.js':[ +31, +], +'project:dojo-1.8.1/dijit/typematic.js.uncompressed.js':[ +15, +], +'project:dojo-1.8.1/dojo/dnd/Moveable.js.uncompressed.js':[ +10, +], +'project:dojo-1.8.1/dojo/dnd/Selector.js.uncompressed.js':[ +28, +], +'project:dojo-1.8.1/dojo/dnd/Source.js.uncompressed.js':[ +68, +], +'project:dojo-1.8.1/dojo/dojo.js.uncompressed.js':[ +2638, +15035, +], +'project:dojo-1.8.1/dojo/dom-form.js.uncompressed.js':[ +28, +], +'project:dojo-1.8.1/dojo/on.js.uncompressed.js':[ +74, +], +'project:dojo-1.8.1/dojo/router/RouterBase.js.uncompressed.js':[ +46, +], +'project:dojo-1.8.1/dojox/atom/io/model.js.uncompressed.js':[ +635, +779, +], +'project:dojo-1.8.1/dojox/av/_Media.js.uncompressed.js':[ +288, +], +'project:dojo-1.8.1/dojox/calendar/SimpleColumnView.js.uncompressed.js':[ +1508, +], +'project:dojo-1.8.1/dojox/charting/plot2d/Stacked.js.uncompressed.js':[ +14, +], +'project:dojo-1.8.1/dojox/charting/themes/Claro.js.uncompressed.js':[ +91, +], +'project:dojo-1.8.1/dojox/css3/transit.js.uncompressed.js':[ +6, +], +'project:dojo-1.8.1/dojox/data/AtomReadStore.js.uncompressed.js':[ +221, +], +'project:dojo-1.8.1/dojox/data/FlickrRestStore.js.uncompressed.js':[ +4, +], +'project:dojo-1.8.1/dojox/data/OpmlStore.js.uncompressed.js':[ +394, +], +'project:dojo-1.8.1/dojox/date/buddhist/locale.js.uncompressed.js':[ +119, +130, +272, +], +'project:dojo-1.8.1/dojox/date/hebrew/locale.js.uncompressed.js':[ +138, +], +'project:dojo-1.8.1/dojox/date/islamic/Date.js.uncompressed.js':[ +336, +], +'project:dojo-1.8.1/dojox/date/islamic/locale.js.uncompressed.js':[ +119, +130, +273, +], +'project:dojo-1.8.1/dojox/dgauges/TextIndicator.js.uncompressed.js':[ +107, +], +'project:dojo-1.8.1/dojox/dgauges/components/default/HorizontalLinearGauge.js.uncompressed.js':[ +52, +], +'project:dojo-1.8.1/dojox/dgauges/components/default/VerticalLinearGauge.js.uncompressed.js':[ +56, +], +'project:dojo-1.8.1/dojox/drawing/Drawing.js.uncompressed.js':[ +323, +], +'project:dojo-1.8.1/dojox/drawing/plugins/drawing/Silverlight.js.uncompressed.js':[ +169, +], +'project:dojo-1.8.1/dojox/flash/_base.js.uncompressed.js':[ +336, +], +'project:dojo-1.8.1/dojox/form/CheckedMultiSelect.js.uncompressed.js':[ +282, +], +'project:dojo-1.8.1/dojox/form/manager/_ClassMixin.js.uncompressed.js':[ +31, +], +'project:dojo-1.8.1/dojox/form/manager/_DisplayMixin.js.uncompressed.js':[ +25, +], +'project:dojo-1.8.1/dojox/fx/_base.js.uncompressed.js':[ +143, +213, +318, +], +'project:dojo-1.8.1/dojox/fx/scroll.js.uncompressed.js':[ +34, +], +'project:dojo-1.8.1/dojox/fx/split.js.uncompressed.js':[ +304, +418, +541, +], +'project:dojo-1.8.1/dojox/fx/text.js.uncompressed.js':[ +219, +286, +305, +347, +361, +449, +], +'project:dojo-1.8.1/dojox/geo/charting/Map.js.uncompressed.js':[ +545, +559, +573, +], +'project:dojo-1.8.1/dojox/geo/openlayers/GfxLayer.js.uncompressed.js':[ +81, +], +'project:dojo-1.8.1/dojox/geo/openlayers/GreatCircle.js.uncompressed.js':[ +7, +87, +], +'project:dojo-1.8.1/dojox/geo/openlayers/JsonImport.js.uncompressed.js':[ +132, +143, +], +'project:dojo-1.8.1/dojox/gesture/Base.js.uncompressed.js':[ +182, +], +'project:dojo-1.8.1/dojox/gfx/arc.js.uncompressed.js':[ +22, +], +'project:dojo-1.8.1/dojox/gfx/svg.js.uncompressed.js':[ +732, +], +'project:dojo-1.8.1/dojox/gfx3d/matrix.js.uncompressed.js':[ +291, +], +'project:dojo-1.8.1/dojox/gfx3d/scheduler.js.uncompressed.js':[ +147, +], +'project:dojo-1.8.1/dojox/grid/_Builder.js.uncompressed.js':[ +433, +538, +], +'project:dojo-1.8.1/dojox/grid/_Selector.js.uncompressed.js':[ +53, +], +'project:dojo-1.8.1/dojox/grid/enhanced/plugins/Cookie.js.uncompressed.js':[ +138, +], +'project:dojo-1.8.1/dojox/grid/enhanced/plugins/filter/FilterDefDialog.js.uncompressed.js':[ +720, +], +'project:dojo-1.8.1/dojox/highlight/languages/pygments/xml.js.uncompressed.js':[ +3, +], +'project:dojo-1.8.1/dojox/io/OAuth.js.uncompressed.js':[ +209, +], +'project:dojo-1.8.1/dojox/jq.js.uncompressed.js':[ +706, +785, +1755, +], +'project:dojo-1.8.1/dojox/lang/docs.js.uncompressed.js':[ +133, +], +'project:dojo-1.8.1/dojox/layout/FloatingPane.js.uncompressed.js':[ +13, +], +'project:dojo-1.8.1/dojox/layout/ScrollPane.js.uncompressed.js':[ +12, +], +'project:dojo-1.8.1/dojox/mobile/_ScrollableMixin.js.uncompressed.js':[ +15, +], +'project:dojo-1.8.1/dojox/mobile/viewRegistry.js.uncompressed.js':[ +10, +], +'project:dojo-1.8.1/dojox/rpc/Service.js.uncompressed.js':[ +312, +], +'project:dojo-1.8.1/dojox/treemap/_utils.js.uncompressed.js':[ +2, +], +'project:dojo-1.8.1/dojox/uuid/_base.js.uncompressed.js':[ +142, +158, +], +'project:dojo-1.8.1/dojox/uuid/generateTimeBasedUuid.js.uncompressed.js':[ +24, +31, +], +'project:dojo-1.8.1/dojox/validate/regexp.js.uncompressed.js':[ +189, +], +'project:dojo-1.8.1/dojox/widget/FilePicker.js.uncompressed.js':[ +165, +], +'project:dojo-1.8.1/dojox/widget/UpgradeBar.js.uncompressed.js':[ +28, +], +'project:dojo-1.8.1/dojox/widget/Wizard.js.uncompressed.js':[ +19, +], +'project:dojo-1.8.1/dojox/widget/WizardPane.js.uncompressed.js':[ +19, +], +'project:dojo-1.8.1/dojox/wire/XmlWire.js.uncompressed.js':[ +71, +], +'project:ecmascript6/Ghost/core/client/app/components/gh-url-preview.js':[ +31, +], +'project:ecmascript6/Ghost/core/client/app/mirage/config.js':[ +124, +245, +], +'project:ecmascript6/Ghost/core/client/app/mixins/ed-editor-scroll.js':[ +31, +], +'project:ecmascript6/Ghost/core/server/api/configuration.js':[ +19, +], +'project:ecmascript6/Ghost/core/server/api/mail.js':[ +111, +], +'project:ecmascript6/Ghost/core/server/storage/local-file-store.js':[ +34, +], +'project:ecmascript6/Ghost/core/server/utils/downzero.js':[ +14, +], +'project:ecmascript6/Ghost/core/server/utils/read-directory.js':[ +35, +], +'project:ecmascript6/di.js/visualize/lib/angular.js':[ +15506, +], +'project:ecmascript6/reddit-mobile/src/server/oauth.es6.js':[ +387, +], +'project:ecmascript6/sonar-web/src/main/js/apps/coding-rules/facets/status-facet.js':[ +9, +], +'project:ecmascript6/watchtower.js/test/watchgroup.spec.js':[ +593, +], +'project:frameworks/wordpress/customize-controls.js':[ +136, +], +'project:google-closure-library/cssom/cssom.js':[ +383, +], +'project:google-closure-library/date/date.js':[ +80, +], +'project:google-closure-library/debug/debug.js':[ +192, +], +'project:google-closure-library/debug/errorreporter.js':[ +164, +], +'project:google-closure-library/dom/dom.js':[ +1432, +], +'project:google-closure-library/editor/plugins/equationeditorbubble.js':[ +123, +], +'project:google-closure-library/events/events.js':[ +240, +], +'project:google-closure-library/format/emailaddress.js':[ +309, +], +'project:google-closure-library/format/format.js':[ +132, +], +'project:google-closure-library/i18n/uchar.js':[ +38, +126, +], +'project:google-closure-library/labs/net/xhr.js':[ +107, +125, +142, +159, +349, +], +'project:google-closure-library/math/long.js':[ +623, +], +'project:google-closure-library/pubsub/pubsub.js':[ +160, +], +'project:google-closure-library/testing/mockclassfactory.js':[ +509, +517, +], +'project:google-closure-library/tweak/tweakui.js':[ +517, +], +'project:google-closure-library/ui/bubble.js':[ +425, +], +'project:google-closure-library/ui/css3menubuttonrenderer.js':[ +78, +], +'project:google-closure-library/ui/equation/imagerenderer.js':[ +108, +], +'project:google-closure-library/ui/imagelessmenubuttonrenderer.js':[ +78, +], +'project:google-closure-library/ui/media/mp3.js':[ +187, +], +'project:google-closure-library/ui/media/photo.js':[ +105, +], +'project:google-closure-library/ui/offlinestatuscard.js':[ +483, +], +'project:google-closure-library/ui/textarea.js':[ +181, +], +'project:google-closure-library/ui/textarearenderer.js':[ +76, +], +'project:mootools-core-1.4.5-full-nocompat.js':[ +771, +1308, +], +'project:ocanvas-2.2.2.js':[ +477, +3220, +], +'project:open-layers-2.12/OpenLayers/Control/MousePosition.js':[ +201, +], +'project:open-layers-2.12/OpenLayers/Control/Panel.js':[ +383, +], +'project:open-layers-2.12/OpenLayers/Format/ArcXML.js':[ +242, +], +'project:open-layers-2.12/OpenLayers/Format/CSWGetDomain/v2_0_2.js':[ +225, +231, +], +'project:open-layers-2.12/OpenLayers/Format/CSWGetRecords/v2_0_2.js':[ +363, +371, +419, +425, +], +'project:open-layers-2.12/OpenLayers/Format/Filter/v1_1_0.js':[ +178, +], +'project:open-layers-2.12/OpenLayers/Format/GeoJSON.js':[ +460, +], +'project:open-layers-2.12/OpenLayers/Format/KML.js':[ +856, +], +'project:open-layers-2.12/OpenLayers/Format/OWSCommon/v1.js':[ +284, +289, +294, +299, +304, +309, +], +'project:open-layers-2.12/OpenLayers/Format/OWSCommon/v1_1_0.js':[ +94, +100, +106, +], +'project:open-layers-2.12/OpenLayers/Format/OWSContext/v0_3_1.js':[ +420, +450, +455, +], +'project:open-layers-2.12/OpenLayers/Format/SOSGetFeatureOfInterest.js':[ +182, +], +'project:open-layers-2.12/OpenLayers/Format/SOSGetObservation.js':[ +293, +], +'project:open-layers-2.12/OpenLayers/Format/WCSGetCoverage.js':[ +124, +130, +136, +], +'project:open-layers-2.12/OpenLayers/Format/WFST/v1.js':[ +289, +418, +], +'project:open-layers-2.12/OpenLayers/Format/WKT.js':[ +143, +], +'project:open-layers-2.12/OpenLayers/Format/WPSExecute.js':[ +190, +], +'project:open-layers-2.12/OpenLayers/Geometry/LineString.js':[ +611, +], +'project:open-layers-2.12/OpenLayers/Geometry/LinearRing.js':[ +358, +], +'project:open-layers-2.12/OpenLayers/Icon.js':[ +236, +], +'project:open-layers-2.12/OpenLayers/Layer/ArcGIS93Rest.js':[ +165, +], +'project:open-layers-2.12/OpenLayers/Layer/Google.js':[ +422, +], +'project:open-layers-2.12/OpenLayers/Layer/MapServer.js':[ +99, +], +'project:open-layers-2.12/OpenLayers/Layer/WMS.js':[ +208, +], +'project:open-layers-2.12/OpenLayers/Map.js':[ +832, +], +'project:open-layers-2.12/OpenLayers/Marker.js':[ +156, +], +'project:open-layers-2.12/OpenLayers/Util.js':[ +787, +1066, +1115, +], +'project:open-layers-2.12/deprecated.js':[ +238, +805, +1043, +4687, +4698, +], +'project:processing-1.3.6.js':[ +8527, +], +'project:yui-3.7.3/calendar-base/calendar-base.js':[ +777, +], +'project:yui-3.7.3/calendar/calendar.js':[ +493, +], +'project:yui-3.7.3/charts-base/charts-base.js':[ +8048, +8604, +8704, +9253, +11497, +12039, +15441, +], +'project:yui-3.7.3/charts-legend/charts-legend.js':[ +1305, +1349, +], +'project:yui-3.7.3/dom-screen/dom-screen.js':[ +55, +65, +], +'project:yui-3.7.3/graphics-canvas/graphics-canvas.js':[ +929, +3388, +], +'project:yui-3.7.3/graphics-svg/graphics-svg.js':[ +2208, +3151, +], +'project:yui-3.7.3/graphics-vml/graphics-vml.js':[ +2344, +3234, +], +'project:yui-3.7.3/io-base/io-base.js':[ +939, +], +'project:yui-3.7.3/io-nodejs/io-nodejs.js':[ +155, +], +'project:yui-3.7.3/matrix/matrix.js':[ +43, +55, +90, +728, +740, +], +'project:yui-3.7.3/node-core/node-core.js':[ +1472, +], +'project:yui-3.7.3/selector-css2/selector-css2.js':[ +34, +], +'project:yui-3.7.3/slider-value-range/slider-value-range.js':[ +164, +], +'project:yui-3.7.3/tabview/tabview.js':[ +398, +], +'project:yui-3.7.3/widget-parent/widget-parent.js':[ +183, +], +} diff --git a/javascript-checks/src/main/java/org/sonar/javascript/checks/CheckList.java b/javascript-checks/src/main/java/org/sonar/javascript/checks/CheckList.java index 4a7869b4b6b..91e1d4a27cc 100644 --- a/javascript-checks/src/main/java/org/sonar/javascript/checks/CheckList.java +++ b/javascript-checks/src/main/java/org/sonar/javascript/checks/CheckList.java @@ -115,6 +115,7 @@ public static List getChecks() { IdChildrenSelectorCheck.class, IdenticalExpressionOnBinaryOperatorCheck.class, IfConditionalAlwaysTrueOrFalseCheck.class, + ImmediatelyReturnedVariableCheck.class, IncrementDecrementInSubExpressionCheck.class, IndexOfCompareToPositiveNumberCheck.class, InOperatorTypeErrorCheck.class, diff --git a/javascript-checks/src/main/java/org/sonar/javascript/checks/ImmediatelyReturnedVariableCheck.java b/javascript-checks/src/main/java/org/sonar/javascript/checks/ImmediatelyReturnedVariableCheck.java new file mode 100644 index 00000000000..a46b5ef28f4 --- /dev/null +++ b/javascript-checks/src/main/java/org/sonar/javascript/checks/ImmediatelyReturnedVariableCheck.java @@ -0,0 +1,110 @@ +/* + * SonarQube JavaScript Plugin + * Copyright (C) 2011-2016 SonarSource SA + * mailto:contact AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +package org.sonar.javascript.checks; + +import java.util.List; +import javax.annotation.Nullable; +import org.sonar.check.Rule; +import org.sonar.javascript.tree.impl.SeparatedList; +import org.sonar.plugins.javascript.api.tree.Tree.Kind; +import org.sonar.plugins.javascript.api.tree.declaration.BindingElementTree; +import org.sonar.plugins.javascript.api.tree.declaration.InitializedBindingElementTree; +import org.sonar.plugins.javascript.api.tree.expression.ExpressionTree; +import org.sonar.plugins.javascript.api.tree.expression.IdentifierTree; +import org.sonar.plugins.javascript.api.tree.statement.BlockTree; +import org.sonar.plugins.javascript.api.tree.statement.ReturnStatementTree; +import org.sonar.plugins.javascript.api.tree.statement.StatementTree; +import org.sonar.plugins.javascript.api.tree.statement.ThrowStatementTree; +import org.sonar.plugins.javascript.api.tree.statement.VariableDeclarationTree; +import org.sonar.plugins.javascript.api.tree.statement.VariableStatementTree; +import org.sonar.plugins.javascript.api.visitors.DoubleDispatchVisitorCheck; + +@Rule(key = "S1488") +public class ImmediatelyReturnedVariableCheck extends DoubleDispatchVisitorCheck { + + private static final String MESSAGE = "Immediately %s this expression instead of assigning it to the temporary variable \"%s\"."; + + @Override + public void visitBlock(BlockTree tree) { + List statements = tree.statements(); + + if (statements.size() > 1) { + + StatementTree lastButOneStatement = statements.get(statements.size() - 2); + StatementTree lastStatement = statements.get(statements.size() - 1); + + if (lastButOneStatement.is(Kind.VARIABLE_STATEMENT)) { + checkStatements(((VariableStatementTree) lastButOneStatement).declaration(), lastStatement); + } + } + + super.visitBlock(tree); + } + + private void checkStatements(VariableDeclarationTree variableDeclaration, StatementTree lastStatement) { + SeparatedList variables = variableDeclaration.variables(); + + if (variables.size() == 1 && variables.get(0).is(Kind.INITIALIZED_BINDING_ELEMENT)) { + InitializedBindingElementTree initializedBindingElementTree = (InitializedBindingElementTree) variables.get(0); + + if (initializedBindingElementTree.left().is(Kind.BINDING_IDENTIFIER)) { + String name = ((IdentifierTree) initializedBindingElementTree.left()).name(); + + if (returnsVariableInLastStatement(lastStatement, name)) { + addIssue(initializedBindingElementTree.right(), String.format(MESSAGE, "return", name)); + + } else if (throwsVariableInLastStatement(lastStatement, name)) { + addIssue(initializedBindingElementTree.right(), String.format(MESSAGE, "throw", name)); + + } + } + } + } + + private static boolean returnsVariableInLastStatement(StatementTree lastStatement, String variableName) { + if (lastStatement.is(Kind.RETURN_STATEMENT)) { + ReturnStatementTree returnStatement = (ReturnStatementTree) lastStatement; + + return isVariable(returnStatement.expression(), variableName); + } + + return false; + } + + private static boolean throwsVariableInLastStatement(StatementTree lastStatement, String variableName) { + if (lastStatement.is(Kind.THROW_STATEMENT)) { + ThrowStatementTree throwStatement = (ThrowStatementTree) lastStatement; + return isVariable(throwStatement.expression(), variableName); + } + + return false; + } + + private static boolean isVariable(@Nullable ExpressionTree expressionTree, String variableName) { + if (expressionTree != null && expressionTree.is(Kind.IDENTIFIER_REFERENCE)) { + String thrownName = ((IdentifierTree) expressionTree).name(); + return thrownName.equals(variableName); + } + + return false; + } + + +} diff --git a/javascript-checks/src/main/resources/org/sonar/l10n/javascript/rules/javascript/S1488.html b/javascript-checks/src/main/resources/org/sonar/l10n/javascript/rules/javascript/S1488.html new file mode 100644 index 00000000000..843d69adb51 --- /dev/null +++ b/javascript-checks/src/main/resources/org/sonar/l10n/javascript/rules/javascript/S1488.html @@ -0,0 +1,18 @@ +

Declaring a variable only to immediately return or throw it is a bad practice.

+

Some developers argue that the practice improves code readability, because it enables them to explicitly name what is being returned. However, this +variable is an internal implementation detail that is not exposed to the callers of the method. The method name should be sufficient for callers to +know exactly what will be returned.

+

Noncompliant Code Example

+
+function computeDurationInMilliseconds() {
+  var duration = (((hours * 60) + minutes) * 60 + seconds ) * 1000 ;
+  return duration;
+}
+
+

Compliant Solution

+
+function computeDurationInMilliseconds() {
+  return (((hours * 60) + minutes) * 60 + seconds ) * 1000 ;
+}
+
+ diff --git a/javascript-checks/src/main/resources/org/sonar/l10n/javascript/rules/javascript/S1488.json b/javascript-checks/src/main/resources/org/sonar/l10n/javascript/rules/javascript/S1488.json new file mode 100644 index 00000000000..c19ff47c4ad --- /dev/null +++ b/javascript-checks/src/main/resources/org/sonar/l10n/javascript/rules/javascript/S1488.json @@ -0,0 +1,13 @@ +{ + "title": "Local Variables should not be declared and then immediately returned or thrown", + "type": "CODE_SMELL", + "status": "ready", + "remediation": { + "func": "Constant\/Issue", + "constantCost": "2min" + }, + "tags": [ + "clumsy" + ], + "defaultSeverity": "Minor" +} diff --git a/javascript-checks/src/main/resources/org/sonar/l10n/javascript/rules/javascript/Sonar_way_profile.json b/javascript-checks/src/main/resources/org/sonar/l10n/javascript/rules/javascript/Sonar_way_profile.json index 820b0294aec..66e8de7f545 100644 --- a/javascript-checks/src/main/resources/org/sonar/l10n/javascript/rules/javascript/Sonar_way_profile.json +++ b/javascript-checks/src/main/resources/org/sonar/l10n/javascript/rules/javascript/Sonar_way_profile.json @@ -41,6 +41,7 @@ "S1301", "S1442", "S1472", + "S1488", "S1656", "S1751", "S1764", diff --git a/javascript-checks/src/test/java/org/sonar/javascript/checks/ImmediatelyReturnedVariableCheckTest.java b/javascript-checks/src/test/java/org/sonar/javascript/checks/ImmediatelyReturnedVariableCheckTest.java new file mode 100644 index 00000000000..1f07b3264bb --- /dev/null +++ b/javascript-checks/src/test/java/org/sonar/javascript/checks/ImmediatelyReturnedVariableCheckTest.java @@ -0,0 +1,32 @@ +/* + * SonarQube JavaScript Plugin + * Copyright (C) 2011-2016 SonarSource SA + * mailto:contact AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +package org.sonar.javascript.checks; + +import java.io.File; +import org.junit.Test; +import org.sonar.javascript.checks.verifier.JavaScriptCheckVerifier; + +public class ImmediatelyReturnedVariableCheckTest { + + @Test + public void test() { + JavaScriptCheckVerifier.verify(new ImmediatelyReturnedVariableCheck(), new File("src/test/resources/checks/ImmediatelyReturnedVariable.js")); + } +} diff --git a/javascript-checks/src/test/resources/checks/ImmediatelyReturnedVariable.js b/javascript-checks/src/test/resources/checks/ImmediatelyReturnedVariable.js new file mode 100644 index 00000000000..ee7f4cea7b6 --- /dev/null +++ b/javascript-checks/src/test/resources/checks/ImmediatelyReturnedVariable.js @@ -0,0 +1,120 @@ +function var_returned() { + var x = 42; // Noncompliant {{Immediately return this expression instead of assigning it to the temporary variable "x".}} +// ^^ + return x; +} + +function let_returned() { + let x = 42; // Noncompliant + return x; +} + +function const_returned() { + const x = 42; // Noncompliant + return x; +} + +function code_before_declaration() { + foo(); + var x = 42; // Noncompliant + return x; +} + +function thrown_nok() { + const x = new Exception(); // Noncompliant {{Immediately throw this expression instead of assigning it to the temporary variable "x".}} + throw x; +} + +function thrown_ok() { + throw new Exception(); +} + +function thrown_expression() { + const x = new Exception(); + throw foo(x); +} + +function thrown_different_variable() { + const x = new Exception(); + throw y; +} + +function code_between_declaration_and_return() { + let x = 42; + foo(); + return x; +} + +function return_expression() { + let x = 42; + return x + 5; +} + +function return_without_value() { + let x = 42; + return; +} + +function not_return_statement() { + let x = 42; + foo(x); +} + +function no_init_value() { + let x; + return x; +} + +function pattern_declared() { + let {x} = foo(); + return x; +} + +function two_variables_declared() { + let x = 42, y; + return x; +} + +function different_variable_returned() { + let x = 42; + return y; +} + +function only_return() { + return 42; +} + +function one_statement() { + foo(); +} + +function empty_block() { +} + +function different_blocks() { + if (foo) { + let x = foo(); // Noncompliant + return x; + } + + try { + let x = foo(); // Noncompliant + return x; + + } catch (e) { + let x = foo(); // Noncompliant + return x; + + } finally { + let x = foo(); // Noncompliant + return x; + } + + +} + +var arrow_function_ok = (a, b) => { + return a + b; +} + +var arrow_function_no_block = (a, b) => a + b;