diff --git a/its/ruling/src/test/resources/expected/commons-beanutils/java-S9438.json b/its/ruling/src/test/resources/expected/commons-beanutils/java-S9438.json new file mode 100644 index 00000000000..4788ecc4b46 --- /dev/null +++ b/its/ruling/src/test/resources/expected/commons-beanutils/java-S9438.json @@ -0,0 +1,7 @@ +{ +"commons-beanutils:src/main/java/org/apache/commons/beanutils2/BeanMap.java": [ +51, +52, +53 +] +} diff --git a/its/ruling/src/test/resources/expected/eclipse-jetty-similar-to-main/java-S9438.json b/its/ruling/src/test/resources/expected/eclipse-jetty-similar-to-main/java-S9438.json new file mode 100644 index 00000000000..af96db1ea4d --- /dev/null +++ b/its/ruling/src/test/resources/expected/eclipse-jetty-similar-to-main/java-S9438.json @@ -0,0 +1,43 @@ +{ +"eclipse-jetty:jetty-http/src/main/java/org/eclipse/jetty/http/HttpCookie.java": [ +60 +], +"eclipse-jetty:jetty-http/src/main/java/org/eclipse/jetty/http/HttpParser.java": [ +171 +], +"eclipse-jetty:jetty-io/src/main/java/org/eclipse/jetty/io/ssl/SslConnection.java": [ +124 +], +"eclipse-jetty:jetty-server/src/main/java/org/eclipse/jetty/server/LowResourceMonitor.java": [ +59, +496 +], +"eclipse-jetty:jetty-server/src/main/java/org/eclipse/jetty/server/MultiPartParser.java": [ +82 +], +"eclipse-jetty:jetty-server/src/main/java/org/eclipse/jetty/server/handler/DebugHandler.java": [ +46 +], +"eclipse-jetty:jetty-server/src/main/java/org/eclipse/jetty/server/handler/InetAccessSet.java": [ +35 +], +"eclipse-jetty:jetty-server/src/main/java/org/eclipse/jetty/server/handler/ThreadLimitHandler.java": [ +335 +], +"eclipse-jetty:jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionContext.java": [ +36, +38, +39, +40 +], +"eclipse-jetty:jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionHandler.java": [ +1067 +], +"eclipse-jetty:jetty-server/src/main/java/org/eclipse/jetty/server/session/UnreadableSessionDataException.java": [ +30, +31 +], +"eclipse-jetty:jetty-server/src/main/java/org/eclipse/jetty/server/session/UnwriteableSessionDataException.java": [ +26 +] +} diff --git a/its/ruling/src/test/resources/expected/eclipse-jetty/java-S9438.json b/its/ruling/src/test/resources/expected/eclipse-jetty/java-S9438.json new file mode 100644 index 00000000000..07f45ab166e --- /dev/null +++ b/its/ruling/src/test/resources/expected/eclipse-jetty/java-S9438.json @@ -0,0 +1,80 @@ +{ +"eclipse-jetty:jetty-http/src/main/java/org/eclipse/jetty/http/HttpCookie.java": [ +60 +], +"eclipse-jetty:jetty-http/src/main/java/org/eclipse/jetty/http/HttpParser.java": [ +171 +], +"eclipse-jetty:jetty-io/src/main/java/org/eclipse/jetty/io/ssl/SslConnection.java": [ +124 +], +"eclipse-jetty:jetty-server/src/main/java/org/eclipse/jetty/server/LowResourceMonitor.java": [ +59, +496 +], +"eclipse-jetty:jetty-server/src/main/java/org/eclipse/jetty/server/MultiPartParser.java": [ +82 +], +"eclipse-jetty:jetty-server/src/main/java/org/eclipse/jetty/server/handler/DebugHandler.java": [ +46 +], +"eclipse-jetty:jetty-server/src/main/java/org/eclipse/jetty/server/handler/InetAccessSet.java": [ +35 +], +"eclipse-jetty:jetty-server/src/main/java/org/eclipse/jetty/server/handler/ThreadLimitHandler.java": [ +335 +], +"eclipse-jetty:jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionContext.java": [ +36, +38, +39, +40 +], +"eclipse-jetty:jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionHandler.java": [ +1067 +], +"eclipse-jetty:jetty-server/src/main/java/org/eclipse/jetty/server/session/UnreadableSessionDataException.java": [ +30, +31 +], +"eclipse-jetty:jetty-server/src/main/java/org/eclipse/jetty/server/session/UnwriteableSessionDataException.java": [ +26 +], +"eclipse-jetty:jetty-util-ajax/src/main/java/org/eclipse/jetty/util/ajax/JSON.java": [ +1593 +], +"eclipse-jetty:jetty-util-ajax/src/main/java/org/eclipse/jetty/util/ajax/JSONEnumConvertor.java": [ +37 +], +"eclipse-jetty:jetty-util/src/main/java/org/eclipse/jetty/util/ClassLoadingObjectInputStream.java": [ +46 +], +"eclipse-jetty:jetty-util/src/main/java/org/eclipse/jetty/util/DecoratedObjectFactory.java": [ +51 +], +"eclipse-jetty:jetty-util/src/main/java/org/eclipse/jetty/util/InetAddressSet.java": [ +41 +], +"eclipse-jetty:jetty-util/src/main/java/org/eclipse/jetty/util/MultiException.java": [ +38 +], +"eclipse-jetty:jetty-util/src/main/java/org/eclipse/jetty/util/MultiPartWriter.java": [ +37 +], +"eclipse-jetty:jetty-util/src/main/java/org/eclipse/jetty/util/QuotedStringTokenizer.java": [ +41, +45 +], +"eclipse-jetty:jetty-util/src/main/java/org/eclipse/jetty/util/security/CertificateValidator.java": [ +60, +61 +], +"eclipse-jetty:jetty-xml/src/main/java/org/eclipse/jetty/xml/XmlParser.java": [ +61, +64, +333, +481, +482, +508 +] +} diff --git a/its/ruling/src/test/resources/expected/guava/java-S9438.json b/its/ruling/src/test/resources/expected/guava/java-S9438.json new file mode 100644 index 00000000000..c9355ee6079 --- /dev/null +++ b/its/ruling/src/test/resources/expected/guava/java-S9438.json @@ -0,0 +1,20 @@ +{ +"guava:src/com/google/common/base/MoreObjects.java": [ +139 +], +"guava:src/com/google/common/base/Objects.java": [ +205 +], +"guava:src/com/google/common/collect/ConcurrentHashMultiset.java": [ +511 +], +"guava:src/com/google/common/collect/ImmutableSortedMap.java": [ +507 +], +"guava:src/com/google/common/io/MultiInputStream.java": [ +36 +], +"guava:src/com/google/common/util/concurrent/SmoothRateLimiter.java": [ +210 +] +} diff --git a/its/ruling/src/test/resources/expected/jboss-ejb3-tutorial/java-S9438.json b/its/ruling/src/test/resources/expected/jboss-ejb3-tutorial/java-S9438.json new file mode 100644 index 00000000000..1141713f78a --- /dev/null +++ b/its/ruling/src/test/resources/expected/jboss-ejb3-tutorial/java-S9438.json @@ -0,0 +1,23 @@ +{ +"jboss-ejb3-tutorial:cachedentity/src/org/jboss/tutorial/cachedentity/bean/EntityTestBean.java": [ +50 +], +"jboss-ejb3-tutorial:injection/src/org/jboss/tutorial/injection/bean/ShoppingCartBean.java": [ +35 +], +"jboss-ejb3-tutorial:jboss_deployment_descriptor/src/org/jboss/tutorial/jboss_deployment_descriptor/bean/ShoppingCartBean.java": [ +29 +], +"jboss-ejb3-tutorial:jndibinding/src/org/jboss/tutorial/jndibinding/bean/CustomerBean.java": [ +46 +], +"jboss-ejb3-tutorial:partial_deployment_descriptor/src/org/jboss/tutorial/partial_deployment_descriptor/bean/ExternalCallbackListener.java": [ +40 +], +"jboss-ejb3-tutorial:stateful/src/org/jboss/tutorial/stateful/bean/ShoppingCartBean.java": [ +35 +], +"jboss-ejb3-tutorial:stateful_deployment_descriptor/src/org/jboss/tutorial/stateful_deployment_descriptor/bean/ShoppingCartBean.java": [ +29 +] +} diff --git a/its/ruling/src/test/resources/expected/mall/java-S9438.json b/its/ruling/src/test/resources/expected/mall/java-S9438.json new file mode 100644 index 00000000000..38351fb730c --- /dev/null +++ b/its/ruling/src/test/resources/expected/mall/java-S9438.json @@ -0,0 +1,313 @@ +{ +"mall:mall-common/src/main/java/com/macro/mall/common/api/ResultCode.java": [ +13, +14 +], +"mall:mall-demo/src/main/java/com/macro/mall/demo/bo/AdminUserDetails.java": [ +16 +], +"mall:mall-mbg/src/main/java/com/macro/mall/model/CmsHelpCategoryExample.java": [ +495, +509 +], +"mall:mall-mbg/src/main/java/com/macro/mall/model/CmsHelpExample.java": [ +556, +570 +], +"mall:mall-mbg/src/main/java/com/macro/mall/model/CmsMemberReportExample.java": [ +626, +640 +], +"mall:mall-mbg/src/main/java/com/macro/mall/model/CmsPrefrenceAreaExample.java": [ +435, +449 +], +"mall:mall-mbg/src/main/java/com/macro/mall/model/CmsPrefrenceAreaProductRelationExample.java": [ +295, +309 +], +"mall:mall-mbg/src/main/java/com/macro/mall/model/CmsSubjectCategoryExample.java": [ +495, +509 +], +"mall:mall-mbg/src/main/java/com/macro/mall/model/CmsSubjectCommentExample.java": [ +566, +580 +], +"mall:mall-mbg/src/main/java/com/macro/mall/model/CmsSubjectExample.java": [ +1066, +1080 +], +"mall:mall-mbg/src/main/java/com/macro/mall/model/CmsSubjectProductRelationExample.java": [ +295, +309 +], +"mall:mall-mbg/src/main/java/com/macro/mall/model/CmsTopicCategoryExample.java": [ +495, +509 +], +"mall:mall-mbg/src/main/java/com/macro/mall/model/CmsTopicCommentExample.java": [ +566, +580 +], +"mall:mall-mbg/src/main/java/com/macro/mall/model/CmsTopicExample.java": [ +806, +820 +], +"mall:mall-mbg/src/main/java/com/macro/mall/model/OmsCartItemExample.java": [ +1277, +1291 +], +"mall:mall-mbg/src/main/java/com/macro/mall/model/OmsCompanyAddressExample.java": [ +785, +799 +], +"mall:mall-mbg/src/main/java/com/macro/mall/model/OmsOrderExample.java": [ +2927, +2941 +], +"mall:mall-mbg/src/main/java/com/macro/mall/model/OmsOrderItemExample.java": [ +1456, +1470 +], +"mall:mall-mbg/src/main/java/com/macro/mall/model/OmsOrderOperateHistoryExample.java": [ +496, +510 +], +"mall:mall-mbg/src/main/java/com/macro/mall/model/OmsOrderReturnApplyExample.java": [ +1887, +1901 +], +"mall:mall-mbg/src/main/java/com/macro/mall/model/OmsOrderReturnReasonExample.java": [ +426, +440 +], +"mall:mall-mbg/src/main/java/com/macro/mall/model/OmsOrderSettingExample.java": [ +475, +489 +], +"mall:mall-mbg/src/main/java/com/macro/mall/model/PmsAlbumExample.java": [ +505, +519 +], +"mall:mall-mbg/src/main/java/com/macro/mall/model/PmsAlbumPicExample.java": [ +305, +319 +], +"mall:mall-mbg/src/main/java/com/macro/mall/model/PmsBrandExample.java": [ +755, +769 +], +"mall:mall-mbg/src/main/java/com/macro/mall/model/PmsCommentExample.java": [ +1016, +1030 +], +"mall:mall-mbg/src/main/java/com/macro/mall/model/PmsCommentReplayExample.java": [ +566, +580 +], +"mall:mall-mbg/src/main/java/com/macro/mall/model/PmsFeightTemplateExample.java": [ +616, +630 +], +"mall:mall-mbg/src/main/java/com/macro/mall/model/PmsMemberPriceExample.java": [ +426, +440 +], +"mall:mall-mbg/src/main/java/com/macro/mall/model/PmsProductAttributeCategoryExample.java": [ +365, +379 +], +"mall:mall-mbg/src/main/java/com/macro/mall/model/PmsProductAttributeExample.java": [ +855, +869 +], +"mall:mall-mbg/src/main/java/com/macro/mall/model/PmsProductAttributeValueExample.java": [ +365, +379 +], +"mall:mall-mbg/src/main/java/com/macro/mall/model/PmsProductCategoryAttributeRelationExample.java": [ +295, +309 +], +"mall:mall-mbg/src/main/java/com/macro/mall/model/PmsProductCategoryExample.java": [ +815, +829 +], +"mall:mall-mbg/src/main/java/com/macro/mall/model/PmsProductExample.java": [ +2517, +2531 +], +"mall:mall-mbg/src/main/java/com/macro/mall/model/PmsProductFullReductionExample.java": [ +356, +370 +], +"mall:mall-mbg/src/main/java/com/macro/mall/model/PmsProductLadderExample.java": [ +416, +430 +], +"mall:mall-mbg/src/main/java/com/macro/mall/model/PmsProductOperateLogExample.java": [ +847, +861 +], +"mall:mall-mbg/src/main/java/com/macro/mall/model/PmsProductVertifyRecordExample.java": [ +496, +510 +], +"mall:mall-mbg/src/main/java/com/macro/mall/model/PmsSkuStockExample.java": [ +806, +820 +], +"mall:mall-mbg/src/main/java/com/macro/mall/model/SmsCouponExample.java": [ +1227, +1241 +], +"mall:mall-mbg/src/main/java/com/macro/mall/model/SmsCouponHistoryExample.java": [ +806, +820 +], +"mall:mall-mbg/src/main/java/com/macro/mall/model/SmsCouponProductCategoryRelationExample.java": [ +435, +449 +], +"mall:mall-mbg/src/main/java/com/macro/mall/model/SmsCouponProductRelationExample.java": [ +435, +449 +], +"mall:mall-mbg/src/main/java/com/macro/mall/model/SmsFlashPromotionExample.java": [ +513, +527 +], +"mall:mall-mbg/src/main/java/com/macro/mall/model/SmsFlashPromotionLogExample.java": [ +556, +570 +], +"mall:mall-mbg/src/main/java/com/macro/mall/model/SmsFlashPromotionProductRelationExample.java": [ +596, +610 +], +"mall:mall-mbg/src/main/java/com/macro/mall/model/SmsFlashPromotionSessionExample.java": [ +513, +527 +], +"mall:mall-mbg/src/main/java/com/macro/mall/model/SmsHomeAdvertiseExample.java": [ +876, +890 +], +"mall:mall-mbg/src/main/java/com/macro/mall/model/SmsHomeBrandExample.java": [ +425, +439 +], +"mall:mall-mbg/src/main/java/com/macro/mall/model/SmsHomeNewProductExample.java": [ +425, +439 +], +"mall:mall-mbg/src/main/java/com/macro/mall/model/SmsHomeRecommendProductExample.java": [ +425, +439 +], +"mall:mall-mbg/src/main/java/com/macro/mall/model/SmsHomeRecommendSubjectExample.java": [ +425, +439 +], +"mall:mall-mbg/src/main/java/com/macro/mall/model/UmsAdminExample.java": [ +776, +790 +], +"mall:mall-mbg/src/main/java/com/macro/mall/model/UmsAdminLoginLogExample.java": [ +506, +520 +], +"mall:mall-mbg/src/main/java/com/macro/mall/model/UmsAdminPermissionRelationExample.java": [ +355, +369 +], +"mall:mall-mbg/src/main/java/com/macro/mall/model/UmsAdminRoleRelationExample.java": [ +295, +309 +], +"mall:mall-mbg/src/main/java/com/macro/mall/model/UmsGrowthChangeHistoryExample.java": [ +616, +630 +], +"mall:mall-mbg/src/main/java/com/macro/mall/model/UmsIntegrationChangeHistoryExample.java": [ +616, +630 +], +"mall:mall-mbg/src/main/java/com/macro/mall/model/UmsIntegrationConsumeSettingExample.java": [ +415, +429 +], +"mall:mall-mbg/src/main/java/com/macro/mall/model/UmsMemberExample.java": [ +1363, +1377 +], +"mall:mall-mbg/src/main/java/com/macro/mall/model/UmsMemberLevelExample.java": [ +916, +930 +], +"mall:mall-mbg/src/main/java/com/macro/mall/model/UmsMemberLoginLogExample.java": [ +566, +580 +], +"mall:mall-mbg/src/main/java/com/macro/mall/model/UmsMemberMemberTagRelationExample.java": [ +295, +309 +], +"mall:mall-mbg/src/main/java/com/macro/mall/model/UmsMemberProductCategoryRelationExample.java": [ +295, +309 +], +"mall:mall-mbg/src/main/java/com/macro/mall/model/UmsMemberReceiveAddressExample.java": [ +785, +799 +], +"mall:mall-mbg/src/main/java/com/macro/mall/model/UmsMemberRuleSettingExample.java": [ +536, +550 +], +"mall:mall-mbg/src/main/java/com/macro/mall/model/UmsMemberStatisticsInfoExample.java": [ +1077, +1091 +], +"mall:mall-mbg/src/main/java/com/macro/mall/model/UmsMemberTagExample.java": [ +366, +380 +], +"mall:mall-mbg/src/main/java/com/macro/mall/model/UmsMemberTaskExample.java": [ +425, +439 +], +"mall:mall-mbg/src/main/java/com/macro/mall/model/UmsMenuExample.java": [ +686, +700 +], +"mall:mall-mbg/src/main/java/com/macro/mall/model/UmsPermissionExample.java": [ +756, +770 +], +"mall:mall-mbg/src/main/java/com/macro/mall/model/UmsResourceCategoryExample.java": [ +366, +380 +], +"mall:mall-mbg/src/main/java/com/macro/mall/model/UmsResourceExample.java": [ +506, +520 +], +"mall:mall-mbg/src/main/java/com/macro/mall/model/UmsRoleExample.java": [ +556, +570 +], +"mall:mall-mbg/src/main/java/com/macro/mall/model/UmsRoleMenuRelationExample.java": [ +295, +309 +], +"mall:mall-mbg/src/main/java/com/macro/mall/model/UmsRolePermissionRelationExample.java": [ +295, +309 +], +"mall:mall-mbg/src/main/java/com/macro/mall/model/UmsRoleResourceRelationExample.java": [ +295, +309 +] +} diff --git a/its/ruling/src/test/resources/expected/sonarqube-6.5/java-S9438.json b/its/ruling/src/test/resources/expected/sonarqube-6.5/java-S9438.json new file mode 100644 index 00000000000..7cf15fc85e6 --- /dev/null +++ b/its/ruling/src/test/resources/expected/sonarqube-6.5/java-S9438.json @@ -0,0 +1,189 @@ +{ +"sonarqube-6.5:src/main/java/org/sonar/ce/queue/CeQueueImpl.java": [ +56 +], +"sonarqube-6.5:src/main/java/org/sonar/server/app/TomcatAccessLog.java": [ +62 +], +"sonarqube-6.5:src/main/java/org/sonar/server/authentication/SsoAuthenticator.java": [ +90 +], +"sonarqube-6.5:src/main/java/org/sonar/server/batch/ProjectDataLoader.java": [ +218, +219, +220, +221 +], +"sonarqube-6.5:src/main/java/org/sonar/server/component/ws/SuggestionsAction.java": [ +93 +], +"sonarqube-6.5:src/main/java/org/sonar/server/computation/task/projectanalysis/analysis/Analysis.java": [ +31, +32, +33 +], +"sonarqube-6.5:src/main/java/org/sonar/server/computation/task/projectanalysis/duplication/DuplicationRepositoryImpl.java": [ +36 +], +"sonarqube-6.5:src/main/java/org/sonar/server/computation/task/projectanalysis/duplication/IntegrateCrossProjectDuplications.java": [ +58 +], +"sonarqube-6.5:src/main/java/org/sonar/server/computation/task/projectanalysis/issue/NewEffortAggregator.java": [ +62, +63 +], +"sonarqube-6.5:src/main/java/org/sonar/server/computation/task/projectanalysis/issue/filter/IssuePattern.java": [ +30, +31 +], +"sonarqube-6.5:src/main/java/org/sonar/server/computation/task/projectanalysis/measure/MeasureRepositoryImpl.java": [ +50 +], +"sonarqube-6.5:src/main/java/org/sonar/server/computation/task/projectanalysis/metric/ReportMetricValidatorImpl.java": [ +37, +38 +], +"sonarqube-6.5:src/main/java/org/sonar/server/computation/task/projectanalysis/qualitymodel/MaintainabilityMeasuresVisitor.java": [ +158, +159 +], +"sonarqube-6.5:src/main/java/org/sonar/server/computation/task/projectanalysis/qualitymodel/NewReliabilityAndSecurityRatingMeasuresVisitor.java": [ +145 +], +"sonarqube-6.5:src/main/java/org/sonar/server/computation/task/projectanalysis/qualitymodel/ReliabilityAndSecurityRatingMeasuresVisitor.java": [ +137 +], +"sonarqube-6.5:src/main/java/org/sonar/server/computation/task/projectanalysis/step/LoadMeasureComputersStep.java": [ +51, +232 +], +"sonarqube-6.5:src/main/java/org/sonar/server/computation/task/projectanalysis/step/PersistFileSourcesStep.java": [ +97 +], +"sonarqube-6.5:src/main/java/org/sonar/server/computation/task/projectanalysis/step/QualityGateMeasuresStep.java": [ +227, +228 +], +"sonarqube-6.5:src/main/java/org/sonar/server/computation/task/projectanalysis/step/SendIssueNotificationsStep.java": [ +57 +], +"sonarqube-6.5:src/main/java/org/sonar/server/debt/DebtModelPluginRepository.java": [ +56 +], +"sonarqube-6.5:src/main/java/org/sonar/server/es/NewIndex.java": [ +203 +], +"sonarqube-6.5:src/main/java/org/sonar/server/es/request/ProxyBulkRequestBuilder.java": [ +126, +127, +128 +], +"sonarqube-6.5:src/main/java/org/sonar/server/issue/IssuesFinderSort.java": [ +36, +37 +], +"sonarqube-6.5:src/main/java/org/sonar/server/issue/notification/ChangesOnMyIssueNotificationDispatcher.java": [ +40 +], +"sonarqube-6.5:src/main/java/org/sonar/server/issue/notification/NewIssuesStatistics.java": [ +41, +42 +], +"sonarqube-6.5:src/main/java/org/sonar/server/issue/workflow/Transition.java": [ +40, +133, +134 +], +"sonarqube-6.5:src/main/java/org/sonar/server/issue/ws/BulkChangeAction.java": [ +364 +], +"sonarqube-6.5:src/main/java/org/sonar/server/measure/index/ProjectMeasuresQuery.java": [ +38 +], +"sonarqube-6.5:src/main/java/org/sonar/server/measure/ws/ComponentTreeData.java": [ +148, +149, +150 +], +"sonarqube-6.5:src/main/java/org/sonar/server/notification/DefaultNotificationManager.java": [ +49, +50, +51 +], +"sonarqube-6.5:src/main/java/org/sonar/server/notification/NotificationDispatcherMetadata.java": [ +41, +42 +], +"sonarqube-6.5:src/main/java/org/sonar/server/notification/email/AlertsEmailTemplate.java": [ +36 +], +"sonarqube-6.5:src/main/java/org/sonar/server/notification/email/EmailNotificationChannel.java": [ +85, +86, +87 +], +"sonarqube-6.5:src/main/java/org/sonar/server/permission/ApplyPermissionTemplateQuery.java": [ +30 +], +"sonarqube-6.5:src/main/java/org/sonar/server/platform/DatabaseServerCompatibility.java": [ +33 +], +"sonarqube-6.5:src/main/java/org/sonar/server/platform/ServerLifecycleNotifier.java": [ +33, +34, +35 +], +"sonarqube-6.5:src/main/java/org/sonar/server/platform/web/MasterServletFilter.java": [ +112, +113 +], +"sonarqube-6.5:src/main/java/org/sonar/server/qualitygate/QgateProjectFinder.java": [ +93, +94 +], +"sonarqube-6.5:src/main/java/org/sonar/server/qualityprofile/QProfileResult.java": [ +28, +29, +33 +], +"sonarqube-6.5:src/main/java/org/sonar/server/qualityprofile/ws/ChangeParentAction.java": [ +44 +], +"sonarqube-6.5:src/main/java/org/sonar/server/qualityprofile/ws/ChangelogAction.java": [ +48 +], +"sonarqube-6.5:src/main/java/org/sonar/server/qualityprofile/ws/ExportersAction.java": [ +30 +], +"sonarqube-6.5:src/main/java/org/sonar/server/rule/ReactivationException.java": [ +26 +], +"sonarqube-6.5:src/main/java/org/sonar/server/source/DecorationDataHolder.java": [ +35, +37 +], +"sonarqube-6.5:src/main/java/org/sonar/server/startup/RenameDeprecatedPropertyKeys.java": [ +33, +34 +], +"sonarqube-6.5:src/main/java/org/sonar/server/user/ExternalIdentity.java": [ +28, +29 +], +"sonarqube-6.5:src/main/java/org/sonar/server/user/NewUser.java": [ +31, +33 +], +"sonarqube-6.5:src/main/java/org/sonar/server/user/NewUserNotifier.java": [ +32 +], +"sonarqube-6.5:src/main/java/org/sonar/server/user/UpdateUser.java": [ +30 +], +"sonarqube-6.5:src/main/java/org/sonar/server/usergroups/ws/UserGroupsWs.java": [ +26 +], +"sonarqube-6.5:src/main/java/org/sonar/server/util/ObjectInputStreamIterator.java": [ +33 +] +} diff --git a/java-checks-test-sources/default/src/main/files/non-compiling/checks/PrivateFieldShouldBeFinalCheckSample.java b/java-checks-test-sources/default/src/main/files/non-compiling/checks/PrivateFieldShouldBeFinalCheckSample.java new file mode 100644 index 00000000000..ae5ad29fa64 --- /dev/null +++ b/java-checks-test-sources/default/src/main/files/non-compiling/checks/PrivateFieldShouldBeFinalCheckSample.java @@ -0,0 +1,42 @@ +package checks; + +class PrivateFieldShouldBeFinalCheckSample { + + // Flexible constructor bodies allow statements before the explicit this(...) invocation + static class DelegatingAfterPrologue { + + private String value; + + DelegatingAfterPrologue(String value) { + this.value = value; + } + + DelegatingAfterPrologue() { + String computed = "default"; + this(computed); + this.value = "overwritten"; + } + + String read() { + return value; + } + } + + static class DelegatingPrologueWithoutWrite { + + private String assignedOnlyInTarget; // Noncompliant + + DelegatingPrologueWithoutWrite(String value) { + this.assignedOnlyInTarget = value; + } + + DelegatingPrologueWithoutWrite() { + String computed = "default"; + this(computed); + } + + String read() { + return assignedOnlyInTarget; + } + } +} diff --git a/java-checks-test-sources/default/src/main/java/checks/PrivateFieldShouldBeFinalCheckSample.java b/java-checks-test-sources/default/src/main/java/checks/PrivateFieldShouldBeFinalCheckSample.java new file mode 100644 index 00000000000..ebd3fe3c4f4 --- /dev/null +++ b/java-checks-test-sources/default/src/main/java/checks/PrivateFieldShouldBeFinalCheckSample.java @@ -0,0 +1,496 @@ +package checks; + +import java.util.ArrayList; +import java.util.List; +import java.util.function.Supplier; +import javax.annotation.Nullable; + +class PrivateFieldShouldBeFinalCheckSample { + + private String assignedInConstructor; // Noncompliant {{Make this private field "final", it is never reassigned.}} [[secondary=41;quickfixes=qf1]] +//^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + // fix@qf1 {{Add the "final" modifier}} + // edit@qf1 [[sc=11;ec=11]] {{final }} + private List assignedAtDeclaration = new ArrayList<>(); // Noncompliant +//^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + private int assignedInInitializerBlock; // Noncompliant [[secondary=37]] +//^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + + private int reassignedInMethod; + private int incrementedInMethod; + private String neverAssigned; + private String assignedTwiceInConstructor; + private String assignedConditionally; + private String assignedInTry; + private String assignedInLambda; + private String lazilyInitialized; + private String assignedInHelperMethod; + private String assignedOnAnotherInstance; + private volatile String volatileField; + private final String alreadyFinal = "final"; + private static String staticField; + String packagePrivate; + protected String protectedField; + public String publicField; + + { + assignedInInitializerBlock = 1; + } + + PrivateFieldShouldBeFinalCheckSample(String value) { + this.assignedInConstructor = value; + this.assignedTwiceInConstructor = value; + this.assignedTwiceInConstructor = value + value; + if (value != null) { + this.assignedConditionally = value; + } + try { + this.assignedInTry = value; + } catch (RuntimeException e) { + this.assignedInTry = ""; + } + Runnable r = () -> this.assignedInLambda = value; + r.run(); + initialize(value); + this.volatileField = value; + PrivateFieldShouldBeFinalCheckSample other = new PrivateFieldShouldBeFinalCheckSample(); + other.assignedOnAnotherInstance = value; + } + + PrivateFieldShouldBeFinalCheckSample() { + this("default"); + } + + private void initialize(String value) { + this.assignedInHelperMethod = value; + } + + void reassign() { + reassignedInMethod = 1; + incrementedInMethod++; + } + + String lazilyInitialized() { + if (lazilyInitialized == null) { + lazilyInitialized = "computed"; + } + return lazilyInitialized; + } + + String readAll() { + return assignedInConstructor + assignedAtDeclaration + assignedInInitializerBlock + neverAssigned + + assignedTwiceInConstructor + assignedConditionally + assignedInTry + assignedInLambda + + assignedInHelperMethod + assignedOnAnotherInstance + alreadyFinal + staticField + + packagePrivate + protectedField + publicField + reassignedInMethod + incrementedInMethod + + volatileField; + } +} + +class AssignedInEveryConstructor { + + private String value; // Noncompliant [[secondary=96,101]] +//^^^^^^^^^^^^^^^^^^^^^ + private String assignedInOnlyOneConstructor; + + AssignedInEveryConstructor() { + this.value = "default"; + this.assignedInOnlyOneConstructor = "default"; + } + + AssignedInEveryConstructor(String value) { + this.value = value; + } + + String read() { + return value + assignedInOnlyOneConstructor; + } +} + +class AssignedInDelegatingConstructor { + + private String assignedInBothConstructors; + private String assignedWithoutBraces; + private int compoundAssigned; + + AssignedInDelegatingConstructor(String value) { + this.assignedInBothConstructors = value; + this.compoundAssigned = 1; + } + + AssignedInDelegatingConstructor() { + this("default"); + this.assignedInBothConstructors = "other"; + } + + AssignedInDelegatingConstructor(int value) { + if (value > 0) this.assignedWithoutBraces = "positive"; + this.assignedInBothConstructors = "int"; + } + + void compound() { + compoundAssigned += 2; + } + + String read() { + return assignedInBothConstructors + assignedWithoutBraces + compoundAssigned; + } +} + +class ConstructorWithEarlyReturn { + + private String guarded; + + ConstructorWithEarlyReturn(String value) { + if (value == null) { + return; + } + this.guarded = value; + } + + String read() { + return guarded; + } +} + +class InitializerBlockOfAnonymousClass { + + private int writtenByAnonymousInitializer; + + Runnable anonymous() { + return new Runnable() { + { + writtenByAnonymousInitializer = 1; + } + + @Override + public void run() { + // nothing + } + }; + } + + int read() { + return writtenByAnonymousInitializer; + } +} + +class ReturnInAnonymousClass { + + private Runnable task; // Noncompliant [[secondary=182]] + + ReturnInAnonymousClass(boolean enabled) { + this.task = new Runnable() { + @Override + public void run() { + if (enabled) { + return; + } + } + }; + } + + Runnable read() { + return task; + } +} + +class EmptyConstructor { + + private String assignedInOneConstructorOnly; + + EmptyConstructor() { + } + + EmptyConstructor(String value) { + this.assignedInOneConstructorOnly = value; + } + + String read() { + return assignedInOneConstructorOnly; + } +} + +class MultiVariableDeclaration { + + private int shared, alsoShared = 1; // Noncompliant [[secondary=218;quickfixes=!]] + + MultiVariableDeclaration(int value) { + this.shared = value; + } + + void reassign() { + alsoShared = 2; + } + + int read() { + return shared + alsoShared; + } +} + +class EmptyAndNonExpressionConstructors { + + private String value; // Noncompliant [[secondary=235,240]] + + EmptyAndNonExpressionConstructors() { + this.value = "empty"; + } + + EmptyAndNonExpressionConstructors(int ignored) { + int local = ignored; + this.value = "local" + local; + } + + String read() { + return value; + } +} + +@javax.persistence.Entity +class PersistenceEntity { + + private String name; + + PersistenceEntity(String name) { + this.name = name; + } + + String read() { + return name; + } +} + +class AnnotatedFields { + + @Nullable + private String annotated; + + AnnotatedFields(String value) { + this.annotated = value; + } + + String read() { + return annotated; + } +} + +enum EnumWithPrivateField { + INSTANCE("name"); + + private String label; // Noncompliant [[secondary=284]] +//^^^^^^^^^^^^^^^^^^^^^ + private int counter; + + EnumWithPrivateField(String label) { + this.label = label; + } + + void increment() { + counter++; + } + + String read() { + return label + counter; + } +} + +class OuterWithNestedWriter { + + private String writtenByNestedClass; + private String writtenByAnonymousClass; + + OuterWithNestedWriter(String value) { + this.writtenByNestedClass = value; + this.writtenByAnonymousClass = value; + } + + class Inner { + void write() { + writtenByNestedClass = "changed"; + } + } + + Supplier anonymous() { + return new Supplier<>() { + @Override + public String get() { + writtenByAnonymousClass = "changed"; + return writtenByAnonymousClass; + } + }; + } + + String read() { + return writtenByNestedClass + writtenByAnonymousClass; + } +} + +interface InterfaceWithField { + String CONSTANT = "constant"; +} + +record RecordSample(String component) { +} + +class UnaryReads { + + private boolean enabled; // Noncompliant + private int offset; // Noncompliant + private int negated; // Noncompliant + + UnaryReads(boolean enabled, int offset, int negated) { + this.enabled = enabled; + this.offset = offset; + this.negated = negated; + } + + boolean read() { + return !enabled && -offset < ~negated; + } +} + +class ReadBeforeAssignment { + + private int readByAnotherFieldInitializer; + private int readEarlierInConstructor; + private int readLaterInConstructor; // Noncompliant + private final int copy = readByAnotherFieldInitializer; + + ReadBeforeAssignment() { + log(readEarlierInConstructor); + this.readEarlierInConstructor = 1; + this.readByAnotherFieldInitializer = 2; + this.readLaterInConstructor = 3; + log(readLaterInConstructor); + } + + private static void log(int value) { + // nothing + } +} + +class ReadInInitializerBlockBeforeWrite { + + private int readBeforeInitializerBlockWrite; + private int alsoRead; + + { + log(readBeforeInitializerBlockWrite); + readBeforeInitializerBlockWrite = 1; + } + + ReadInInitializerBlockBeforeWrite() { + log(alsoRead); + this.alsoRead = 2; + } + + private static void log(int value) { + // nothing + } +} + +class ReadFromNestedClassInitializer { + + private int readByAnonymousClass; // Noncompliant + private int readByInnerClass; // Noncompliant + + ReadFromNestedClassInitializer(int value) { + this.readByAnonymousClass = value; + this.readByInnerClass = value; + } + + Object anonymous() { + return new Object() { + int copy = readByAnonymousClass; + }; + } + + class Inner { + int copy; + + { + copy = readByInnerClass; + } + } +} + +class CopyConstructor { + + private int coordinate; // Noncompliant + + CopyConstructor(CopyConstructor other) { + this.coordinate = other.coordinate; + } + + int read() { + return coordinate; + } +} + +class SelfReferencingAssignment { + + private int incrementedInConstructor; + private int incrementedInInitializerBlock; + + { + incrementedInInitializerBlock = incrementedInInitializerBlock + 1; + } + + SelfReferencingAssignment() { + this.incrementedInConstructor = this.incrementedInConstructor + 1; + } + + int read() { + return incrementedInConstructor + incrementedInInitializerBlock; + } +} + +class ReadInDelegatingConstructor { + + private String value; // Noncompliant + + ReadInDelegatingConstructor(String value) { + this.value = value; + } + + ReadInDelegatingConstructor() { + this("default"); + log(value); + } + + private static void log(String value) { + // nothing + } +} + +class CStyleArrayDeclaration { + + private int sharedWithArrayFragment[], alsoSharedWithArrayFragment; // Noncompliant + + CStyleArrayDeclaration(int value) { + this.sharedWithArrayFragment = new int[value]; + } + + void reassign() { + alsoSharedWithArrayFragment = 2; + } + + int read() { + return sharedWithArrayFragment.length + alsoSharedWithArrayFragment; + } +} + +class ParenthesizedWrites { + + private int incrementedInParentheses; + private int assignedInParentheses; + + ParenthesizedWrites() { + this.incrementedInParentheses = 0; + this.assignedInParentheses = 0; + } + + void mutate() { + (incrementedInParentheses)++; + (this.assignedInParentheses) = 2; + } +} diff --git a/java-checks/src/main/java/org/sonar/java/checks/PrivateFieldShouldBeFinalCheck.java b/java-checks/src/main/java/org/sonar/java/checks/PrivateFieldShouldBeFinalCheck.java new file mode 100644 index 00000000000..b8ad02a8864 --- /dev/null +++ b/java-checks/src/main/java/org/sonar/java/checks/PrivateFieldShouldBeFinalCheck.java @@ -0,0 +1,461 @@ +/* + * SonarQube Java + * Copyright (C) SonarSource Sàrl + * mailto:info AT sonarsource DOT com + * + * You can redistribute and/or modify this program under the terms of + * the Sonar Source-Available License Version 1, as published by SonarSource Sàrl. + * + * 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 Sonar Source-Available License for more details. + * + * You should have received a copy of the Sonar Source-Available License + * along with this program; if not, see https://sonarsource.com/license/ssal/ + */ +package org.sonar.java.checks; + +import java.util.ArrayList; +import java.util.List; +import javax.annotation.CheckForNull; +import org.sonar.check.Rule; +import org.sonar.java.checks.helpers.QuickFixHelper; +import org.sonar.java.model.ExpressionUtils; +import org.sonar.java.reporting.JavaQuickFix; +import org.sonar.java.reporting.JavaTextEdit; +import org.sonar.plugins.java.api.IssuableSubscriptionVisitor; +import org.sonar.plugins.java.api.location.Position; +import org.sonar.plugins.java.api.JavaFileScannerContext; +import org.sonar.plugins.java.api.semantic.Symbol; +import org.sonar.plugins.java.api.tree.AssignmentExpressionTree; +import org.sonar.plugins.java.api.tree.BaseTreeVisitor; +import org.sonar.plugins.java.api.tree.BlockTree; +import org.sonar.plugins.java.api.tree.ClassTree; +import org.sonar.plugins.java.api.tree.ExpressionStatementTree; +import org.sonar.plugins.java.api.tree.ExpressionTree; +import org.sonar.plugins.java.api.tree.IdentifierTree; +import org.sonar.plugins.java.api.tree.LambdaExpressionTree; +import org.sonar.plugins.java.api.tree.MemberSelectExpressionTree; +import org.sonar.plugins.java.api.tree.MethodInvocationTree; +import org.sonar.plugins.java.api.tree.MethodTree; +import org.sonar.plugins.java.api.tree.ReturnStatementTree; +import org.sonar.plugins.java.api.tree.StatementTree; +import org.sonar.plugins.java.api.tree.Tree; +import org.sonar.plugins.java.api.tree.UnaryExpressionTree; +import org.sonar.plugins.java.api.tree.VariableTree; + +@Rule(key = "S9438") +public class PrivateFieldShouldBeFinalCheck extends IssuableSubscriptionVisitor { + + private static final String MESSAGE = "Make this private field \"final\", it is never reassigned."; + private static final String SECONDARY_MESSAGE = "Assigned during initialization."; + private static final String QUICK_FIX_MESSAGE = "Add the \"final\" modifier"; + + private static final List PERSISTENCE_ANNOTATIONS = List.of( + "javax.persistence.Entity", + "javax.persistence.Embeddable", + "javax.persistence.MappedSuperclass", + "jakarta.persistence.Entity", + "jakarta.persistence.Embeddable", + "jakarta.persistence.MappedSuperclass"); + + @Override + public List nodesToVisit() { + return List.of(Tree.Kind.CLASS, Tree.Kind.ENUM); + } + + @Override + public void visitNode(Tree tree) { + if (context.getSemanticModel() == null) { + return; + } + ClassTree classTree = (ClassTree) tree; + if (isManagedByPersistenceFramework(classTree)) { + return; + } + List constructors = constructorsOf(classTree); + for (Tree member : classTree.members()) { + if (member.is(Tree.Kind.VARIABLE)) { + checkField((VariableTree) member, classTree, constructors); + } + } + } + + private void checkField(VariableTree field, ClassTree classTree, List constructors) { + if (!isCandidate(field)) { + return; + } + InitializationWrites writes = collectWrites(field, classTree, constructors); + if (!writes.makesFinalPossible(field, constructors) || isReadBeforeInitialization(field, classTree, writes)) { + return; + } + QuickFixHelper.newIssue(context) + .forRule(this) + .onTree(field) + .withMessage(MESSAGE) + .withSecondaries(writes.secondaries()) + .withQuickFixes(() -> quickFixes(field, classTree)) + .report(); + } + + /** + * Every fragment of a multi-variable declaration such as {@code private int a, b;} shares one modifiers tree, + * so inserting the modifier there would make all of them final, which may not compile. + */ + private static List quickFixes(VariableTree field, ClassTree classTree) { + boolean sharesDeclaration = classTree.members().stream() + .filter(member -> member.is(Tree.Kind.VARIABLE)) + .map(VariableTree.class::cast) + .anyMatch(other -> other != field && other.modifiers() == field.modifiers()); + if (sharesDeclaration) { + return List.of(); + } + return List.of(JavaQuickFix.newQuickFix(QUICK_FIX_MESSAGE) + .addTextEdit(JavaTextEdit.insertBeforeTree(field.type(), "final ")) + .build()); + } + + private static boolean isCandidate(VariableTree field) { + Symbol symbol = field.symbol(); + return symbol.isVariableSymbol() + && !symbol.isUnknown() + && symbol.isPrivate() + && !symbol.isStatic() + && !symbol.isFinal() + && !symbol.isVolatile() + && field.modifiers().annotations().isEmpty(); + } + + /** + * Fields of persistence entities are written reflectively by the framework, outside of any constructor, + * so they cannot be made final even when the analyzed code never reassigns them. + */ + private static boolean isManagedByPersistenceFramework(ClassTree classTree) { + return PERSISTENCE_ANNOTATIONS.stream().anyMatch(classTree.symbol().metadata()::isAnnotatedWith); + } + + private static InitializationWrites collectWrites(VariableTree field, ClassTree classTree, List constructors) { + InitializationWrites writes = new InitializationWrites(); + for (IdentifierTree usage : field.symbol().usages()) { + AssignmentExpressionTree assignment = plainAssignmentTo(usage); + if (assignment == null) { + if (isUnsupportedWrite(usage)) { + writes.disqualify(); + return writes; + } + continue; + } + MethodTree constructor = enclosingConstructor(assignment, constructors); + if (constructor != null) { + writes.addConstructorWrite(constructor, assignment); + } else if (isDirectlyInInstanceInitializer(assignment, classTree)) { + writes.addInitializerBlockWrite(assignment); + } else { + writes.disqualify(); + return writes; + } + } + return writes; + } + + /** + * Returns the simple assignment the usage is the target of, or null when the usage is not such a target. + */ + @CheckForNull + private static AssignmentExpressionTree plainAssignmentTo(IdentifierTree usage) { + Tree parent = enclosingWriteExpression(usage); + if (parent instanceof AssignmentExpressionTree assignment + && parent.is(Tree.Kind.ASSIGNMENT) + && isOnThisInstance(assignment.variable())) { + return assignment; + } + return null; + } + + /** + * Compound assignments and increments read the field before writing it, so they are never compatible with final, + * and an assignment on another instance proves the field is mutated after construction. + */ + private static boolean isUnsupportedWrite(IdentifierTree usage) { + return enclosingWriteExpression(usage) != null; + } + + /** + * Returns the expression writing to the given usage, be it an assignment, a compound assignment or an + * increment, or null when the usage only reads the field. + */ + @CheckForNull + private static Tree enclosingWriteExpression(IdentifierTree usage) { + Tree target = usage; + Tree parent = usage.parent(); + if (parent != null && parent.is(Tree.Kind.MEMBER_SELECT) && ((MemberSelectExpressionTree) parent).identifier() == usage) { + target = parent; + parent = parent.parent(); + } + while (parent != null && parent.is(Tree.Kind.PARENTHESIZED_EXPRESSION)) { + target = parent; + parent = parent.parent(); + } + if (parent instanceof AssignmentExpressionTree assignment && assignment.variable() == target) { + return parent; + } + if (parent instanceof UnaryExpressionTree unary && unary.expression() == target && isIncrementOrDecrement(parent)) { + return parent; + } + return null; + } + + private static boolean isIncrementOrDecrement(Tree tree) { + return tree.is(Tree.Kind.PREFIX_INCREMENT, Tree.Kind.PREFIX_DECREMENT, Tree.Kind.POSTFIX_INCREMENT, Tree.Kind.POSTFIX_DECREMENT); + } + + /** + * A blank final field must be definitely assigned before it is read by its simple name, so a read that + * happens earlier than the initialization would no longer compile once the modifier is added. + */ + private static boolean isReadBeforeInitialization(VariableTree field, ClassTree classTree, InitializationWrites writes) { + return field.symbol().usages().stream() + .filter(usage -> enclosingWriteExpression(usage) == null) + .filter(PrivateFieldShouldBeFinalCheck::isAccessOnThisInstance) + .anyMatch(read -> readPrecedesInitialization(read, field, classTree, writes)); + } + + private static boolean isAccessOnThisInstance(IdentifierTree usage) { + Tree parent = usage.parent(); + if (parent instanceof MemberSelectExpressionTree select && select.identifier() == usage) { + return ExpressionUtils.isThis(select.expression()); + } + return true; + } + + private static boolean readPrecedesInitialization(IdentifierTree read, VariableTree field, ClassTree classTree, InitializationWrites writes) { + Tree scope = enclosingExecutableScope(read); + if (scope.is(Tree.Kind.CONSTRUCTOR)) { + AssignmentExpressionTree assignment = writes.assignmentIn((MethodTree) scope); + return assignment != null && startsBefore(read, assignment); + } + if (!initializesDeclaringClass(scope, classTree)) { + return false; + } + // the read happens while the class is being initialized, that is before any constructor body + if (writes.isAssignedInConstructor()) { + return true; + } + Tree write = field.initializer() != null ? field.initializer() : writes.firstAssignment(); + return write != null && startsBefore(read, write); + } + + /** + * A field initializer or an instance initializer block of a nested, local or anonymous class runs when that + * other class is instantiated, which is always after the declaring class has been fully constructed. + */ + private static boolean initializesDeclaringClass(Tree scope, ClassTree classTree) { + return scope == classTree || (scope.is(Tree.Kind.INITIALIZER) && scope.parent() == classTree); + } + + /** + * Returns the method, constructor or instance initializer block the tree belongs to, or the class itself + * when the tree belongs to a field initializer. + */ + private static Tree enclosingExecutableScope(Tree tree) { + Tree scope = tree; + for (Tree parent = tree.parent(); parent != null; parent = parent.parent()) { + scope = parent; + if (parent.is(Tree.Kind.METHOD, Tree.Kind.CONSTRUCTOR, Tree.Kind.INITIALIZER, Tree.Kind.STATIC_INITIALIZER) + || parent instanceof ClassTree) { + break; + } + } + return scope; + } + + /** + * A read nested in the right-hand side of the initializing write still happens before the field is assigned, + * hence the comparison with the end of the write and not with its start. + */ + private static boolean startsBefore(Tree read, Tree write) { + return Position.startOf(read).compareTo(Position.endOf(write)) < 0; + } + + private static boolean isOnThisInstance(ExpressionTree variable) { + ExpressionTree unwrapped = ExpressionUtils.skipParentheses(variable); + if (unwrapped.is(Tree.Kind.MEMBER_SELECT)) { + return ExpressionUtils.isThis(((MemberSelectExpressionTree) unwrapped).expression()); + } + return true; + } + + /** + * A final field can only be assigned by a statement that always runs exactly once, which means a statement + * directly in the body of a constructor, not nested in any conditional, loop, lambda or inner class. + * A constructor that can return early is rejected, because the field would not be assigned on that path. + */ + @CheckForNull + private static MethodTree enclosingConstructor(AssignmentExpressionTree assignment, List constructors) { + BlockTree body = enclosingStatementBlock(assignment); + if (body == null || !body.is(Tree.Kind.BLOCK)) { + return null; + } + Tree owner = body.parent(); + if (owner != null && owner.is(Tree.Kind.CONSTRUCTOR) && constructors.contains(owner) && !canReturnEarly((MethodTree) owner)) { + return (MethodTree) owner; + } + return null; + } + + private static boolean isDirectlyInInstanceInitializer(AssignmentExpressionTree assignment, ClassTree classTree) { + BlockTree block = enclosingStatementBlock(assignment); + return block != null && block.is(Tree.Kind.INITIALIZER) && block.parent() == classTree; + } + + @CheckForNull + private static BlockTree enclosingStatementBlock(AssignmentExpressionTree assignment) { + Tree statement = assignment.parent(); + if (statement == null || !statement.is(Tree.Kind.EXPRESSION_STATEMENT)) { + return null; + } + Tree block = statement.parent(); + if (block instanceof BlockTree blockTree) { + return blockTree; + } + return null; + } + + private static boolean canReturnEarly(MethodTree constructor) { + ReturnStatementVisitor visitor = new ReturnStatementVisitor(); + constructor.accept(visitor); + return visitor.found; + } + + private static List constructorsOf(ClassTree classTree) { + return classTree.members().stream() + .filter(member -> member.is(Tree.Kind.CONSTRUCTOR)) + .map(MethodTree.class::cast) + .toList(); + } + + /** + * Finds return statements belonging to the visited method itself, skipping the bodies of lambdas, + * anonymous classes and nested classes, whose returns do not affect the enclosing constructor. + */ + private static class ReturnStatementVisitor extends BaseTreeVisitor { + + private boolean found; + + @Override + public void visitReturnStatement(ReturnStatementTree tree) { + found = true; + } + + @Override + public void visitLambdaExpression(LambdaExpressionTree tree) { + // a return inside a lambda leaves the lambda, not the constructor + } + + @Override + public void visitClass(ClassTree tree) { + // a return inside a local, nested or anonymous class belongs to one of its own methods + } + } + + private static class InitializationWrites { + + private final List assigningConstructors = new ArrayList<>(); + private final List constructorAssignments = new ArrayList<>(); + private final List assignments = new ArrayList<>(); + private int initializerBlockWrites; + private boolean disqualified; + + void disqualify() { + disqualified = true; + } + + void addConstructorWrite(MethodTree constructor, AssignmentExpressionTree assignment) { + assigningConstructors.add(constructor); + constructorAssignments.add(assignment); + assignments.add(assignment); + } + + void addInitializerBlockWrite(AssignmentExpressionTree assignment) { + initializerBlockWrites++; + assignments.add(assignment); + } + + boolean isAssignedInConstructor() { + return !assigningConstructors.isEmpty(); + } + + @CheckForNull + AssignmentExpressionTree firstAssignment() { + return assignments.isEmpty() ? null : assignments.get(0); + } + + @CheckForNull + AssignmentExpressionTree assignmentIn(MethodTree constructor) { + for (int i = 0; i < assigningConstructors.size(); i++) { + if (assigningConstructors.get(i) == constructor) { + return constructorAssignments.get(i); + } + } + return null; + } + + List secondaries() { + return assignments.stream() + .map(assignment -> new JavaFileScannerContext.Location(SECONDARY_MESSAGE, assignment)) + .toList(); + } + + /** + * The field can be made final when it is assigned exactly once on every path reaching the end of construction: + * either by its own initializer, or by a single instance initializer block, or by every constructor that does + * not delegate to another one. A constructor delegating with {@code this(...)} must not assign the field at all, + * since the constructor it delegates to already does. + */ + boolean makesFinalPossible(VariableTree field, List constructors) { + if (disqualified) { + return false; + } + if (field.initializer() != null) { + return assignments.isEmpty(); + } + if (initializerBlockWrites > 0) { + return initializerBlockWrites == 1 && assigningConstructors.isEmpty(); + } + if (assigningConstructors.stream().anyMatch(InitializationWrites::delegatesToAnotherConstructor)) { + return false; + } + List required = constructors.stream() + .filter(constructor -> !delegatesToAnotherConstructor(constructor)) + .toList(); + return !required.isEmpty() + && required.stream().allMatch(this::assignedExactlyOnceIn); + } + + private boolean assignedExactlyOnceIn(MethodTree constructor) { + return assigningConstructors.stream().filter(constructor::equals).count() == 1; + } + + /** + * Flexible constructor bodies allow statements before the explicit constructor invocation, + * so the {@code this(...)} call is not necessarily the first statement. + */ + private static boolean delegatesToAnotherConstructor(MethodTree constructor) { + BlockTree block = constructor.block(); + return block != null && block.body().stream().anyMatch(InitializationWrites::isThisInvocation); + } + + private static boolean isThisInvocation(StatementTree statement) { + if (!statement.is(Tree.Kind.EXPRESSION_STATEMENT)) { + return false; + } + ExpressionTree expression = ((ExpressionStatementTree) statement).expression(); + if (!expression.is(Tree.Kind.METHOD_INVOCATION)) { + return false; + } + ExpressionTree methodSelect = ((MethodInvocationTree) expression).methodSelect(); + return methodSelect.is(Tree.Kind.IDENTIFIER) && "this".equals(((IdentifierTree) methodSelect).name()); + } + } +} diff --git a/java-checks/src/main/java/org/sonar/java/filters/LombokFilter.java b/java-checks/src/main/java/org/sonar/java/filters/LombokFilter.java index ab82c5ed588..084e8605729 100644 --- a/java-checks/src/main/java/org/sonar/java/filters/LombokFilter.java +++ b/java-checks/src/main/java/org/sonar/java/filters/LombokFilter.java @@ -29,6 +29,7 @@ import org.sonar.java.checks.EqualsNotOverriddenWithCompareToCheck; import org.sonar.java.checks.ExceptionsShouldBeImmutableCheck; import org.sonar.java.checks.FieldModifierCheck; +import org.sonar.java.checks.PrivateFieldShouldBeFinalCheck; import org.sonar.java.checks.PrivateFieldUsedLocallyCheck; import org.sonar.java.checks.RegexPatternsNeedlesslyCheck; import org.sonar.java.checks.SillyEqualsCheck; @@ -66,6 +67,7 @@ public class LombokFilter extends BaseTreeVisitorIssueFilter { /* S1165 */ ExceptionsShouldBeImmutableCheck.class, /* S2039 */ FieldModifierCheck.class, /* S1450 */ PrivateFieldUsedLocallyCheck.class, + /* S9438 */ PrivateFieldShouldBeFinalCheck.class, /* S4248 */ RegexPatternsNeedlesslyCheck.class, /* S2159 */ SillyEqualsCheck.class, /* S2325 */ StaticMethodCheck.class, @@ -105,6 +107,12 @@ public class LombokFilter extends BaseTreeVisitorIssueFilter { LOMBOK_DATA, LOMBOK_VALUE); + private static final List GENERATE_SETTERS = List.of( + "lombok.Setter", + LOMBOK_DATA, + LOMBOK_BUILDER, + LOMBOK_SUPER_BUILDER); + private static final List UTILITY_CLASS = Collections.singletonList("lombok.experimental.UtilityClass"); private static final List NON_FINAL = Collections.singletonList("lombok.experimental.NonFinal"); @@ -129,6 +137,8 @@ public void visitClass(ClassTree tree) { excludeLinesIfTrue(generatesEquals || usesAnnotation(tree, GENERATE_UNUSED_FIELD_RELATED_METHODS), tree, UnusedPrivateFieldCheck.class, PrivateFieldUsedLocallyCheck.class); excludeLinesIfTrue(usesAnnotation(tree, GENERATE_CONSTRUCTOR), tree, AtLeastOneConstructorCheck.class); + excludeLinesIfTrue(usesAnnotation(tree, GENERATE_SETTERS) || usesAnnotation(tree, GENERATE_CONSTRUCTOR) || generatesFinalFields(tree), + tree, PrivateFieldShouldBeFinalCheck.class); excludeLinesIfTrue(generatesEquals, tree, EqualsNotOverriddenInSubclassCheck.class, EqualsNotOverriddenWithCompareToCheck.class); excludeLinesIfTrue(generatesNonPublicConstructor(tree), tree, UtilityClassWithPublicConstructorCheck.class); boolean isUtilityClass = usesAnnotation(tree, UTILITY_CLASS); diff --git a/java-checks/src/test/files/filters/LombokFilter.java b/java-checks/src/test/files/filters/LombokFilter.java index 34b8f48dea7..8e73c820d12 100644 --- a/java-checks/src/test/files/filters/LombokFilter.java +++ b/java-checks/src/test/files/filters/LombokFilter.java @@ -86,6 +86,61 @@ class Builder5 { // WithIssue public final int foo = 1; // WithIssue } + // A generated setter reassigns the field, so it cannot be made final + @lombok.Setter + class SetterAssignedInConstructor { // WithIssue + private int foo; // NoIssue + + SetterAssignedInConstructor(int foo) { + this.foo = foo; + } + + int read() { + return foo; + } + } + + // A generated no-arg constructor leaves the field unassigned, so it cannot be made final + @lombok.NoArgsConstructor + class NoArgsConstructorAssignedInConstructor { // WithIssue + private int foo; // NoIssue + + NoArgsConstructorAssignedInConstructor(int foo) { + this.foo = foo; + } + + int read() { + return foo; + } + } + + // Lombok already makes the fields of a @Value class final + @lombok.Value + class ValueAssignedInConstructor { // NoIssue + private String foo; // NoIssue + + ValueAssignedInConstructor(String foo) { + this.foo = foo; + } + + String read() { + return foo; + } + } + + // Without a generated setter, a field assigned only in the constructor should be made final + class SetterAssignedInConstructor2 { // WithIssue + private int foo; // WithIssue + + SetterAssignedInConstructor2(int foo) { + this.foo = foo; + } + + int read() { + return foo; + } + } + @lombok.ToString class ToString { // WithIssue private int foo; // NoIssue diff --git a/java-checks/src/test/java/org/sonar/java/checks/PrivateFieldShouldBeFinalCheckTest.java b/java-checks/src/test/java/org/sonar/java/checks/PrivateFieldShouldBeFinalCheckTest.java new file mode 100644 index 00000000000..88907568831 --- /dev/null +++ b/java-checks/src/test/java/org/sonar/java/checks/PrivateFieldShouldBeFinalCheckTest.java @@ -0,0 +1,55 @@ +/* + * SonarQube Java + * Copyright (C) SonarSource Sàrl + * mailto:info AT sonarsource DOT com + * + * You can redistribute and/or modify this program under the terms of + * the Sonar Source-Available License Version 1, as published by SonarSource Sàrl. + * + * 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 Sonar Source-Available License for more details. + * + * You should have received a copy of the Sonar Source-Available License + * along with this program; if not, see https://sonarsource.com/license/ssal/ + */ +package org.sonar.java.checks; + +import org.junit.jupiter.api.Test; +import org.sonar.java.checks.verifier.CheckVerifier; + +import static org.sonar.java.checks.verifier.TestUtils.mainCodeSourcesPath; +import static org.sonar.java.checks.verifier.TestUtils.nonCompilingTestSourcesPath; + +class PrivateFieldShouldBeFinalCheckTest { + + private static final String SAMPLE = "checks/PrivateFieldShouldBeFinalCheckSample.java"; + private static final PrivateFieldShouldBeFinalCheck CHECK = new PrivateFieldShouldBeFinalCheck(); + + @Test + void test() { + CheckVerifier.newVerifier() + .onFile(mainCodeSourcesPath(SAMPLE)) + .withCheck(CHECK) + .verifyIssues(); + } + + @Test + void test_flexible_constructor_bodies() { + CheckVerifier.newVerifier() + .onFile(nonCompilingTestSourcesPath(SAMPLE)) + .withCheck(CHECK) + .withJavaVersion(25) + .verifyIssues(); + } + + @Test + void test_without_semantic() { + CheckVerifier.newVerifier() + .onFile(mainCodeSourcesPath(SAMPLE)) + .withCheck(CHECK) + .withoutSemantic() + .verifyNoIssues(); + } +} diff --git a/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9438.html b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9438.html new file mode 100644 index 00000000000..0a71f05d93b --- /dev/null +++ b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9438.html @@ -0,0 +1,109 @@ +

This rule raises an issue on a private, non-final instance field whose value is assigned only during object +initialization and never reassigned afterwards. Such a field is already immutable in practice, and declaring it final states that +contract in the code instead of leaving it to be rediscovered.

+

Why is this an issue?

+

A private instance field whose only assignments happen during object initialization, in its declaration, in an instance initializer block, or in a +constructor, never changes value once the constructor returns. As long as it is not declared final, nothing says so:

+
    +
  • A reader who needs to know whether the value can change has to scan the whole class for another assignment.
    • +
  • A later change can reassign the field by mistake, and the compiler will accept it. With final, the same mistake does not + compile.
    • +
  • The field has no safe-publication guarantee. A final field is guaranteed to be visible, fully initialized, to any thread that reads + the object after construction, even if the reference was published without synchronization.
    • +
+

Note that final is shallow: it freezes the reference, not the object behind it. A final List field can still have +elements added to it, and a final field holding a mutable object says nothing about that object’s state. Use final to +express "this field always points at the same value", not "this object cannot change".

+

Exceptions

+

No issue is raised in the following cases.

+
    +
  • Fields that are not private. Proving that a package-private, protected or public field is never + reassigned would require analyzing every class that can see it, so the rule deliberately stops at private.
    • +
  • Fields assigned by a framework through reflection, outside any constructor. Adding final to those fields breaks the framework or + the build. This covers at least: +
      +
    • JPA and Hibernate entity fields, for example fields in a type annotated with @Entity or @Embeddable, and fields + annotated with @Id, @Column, @ManyToOne or @OneToMany.
      • +
    • Deserialization targets, for example fields annotated with @JsonProperty, @JsonCreator, @XmlElement + or @JsonbProperty.
      • +
    • Injected fields, for example fields annotated with @Autowired, @Value, @Inject, + @Resource or @PersistenceContext.
      • +
    • Lombok-generated code, for example fields in a type annotated with @Setter, @Data or @Builder.
      • +
    • Test doubles, for example fields annotated with @Mock, @Spy, @InjectMocks or + @Captor.
      • +
    • +
  • Fields that are not assigned exactly once by a statement placed directly in the body of every constructor that does not delegate to another one + with this(…​). This covers fields assigned inside a conditional, a loop or a try block, and fields assigned in a + constructor that can return early. A final field must be definitely assigned by the end of each constructor, and the rule does not + follow the control flow to prove it in these cases.
    • +
  • Lazily initialized fields, assigned on first use rather than during construction, including caches and memoized values.
    • +
  • Fields reassigned through a reference to the object under construction that escapes the constructor, for example a field written by a callback + invoked from the constructor.
    • +
+

How to fix it

+

Add the final modifier to the field declaration. The change preserves behavior: if the code compiles afterwards, the field was +effectively final already, and final only documents it and locks it in.

+

If the compiler then rejects the field because it is "possibly already assigned" or "might not have been initialized", the field is not effectively +final after all, and the issue should be reviewed instead of fixed. That happens when an assignment was missed on a constructor path or when the field +is assigned more than once.

+

An issue on a field that a framework writes by reflection through an annotation this rule does not know about is a false positive and should be +reported as such. An issue on a field that is effectively final today but is expected to become mutable as the class evolves is a genuine match, and +leaving it as-is is a deliberate trade-off rather than a false positive.

+

Do not reach for final as a way to make a class immutable. A field pointing at a mutable object stays mutable through that object, so +also consider defensive copying or an unmodifiable view if immutability is the goal.

+

Code examples

+

Noncompliant code example

+
+public class ShipmentTracker {
+
+  private String carrier; // Noncompliant: assigned only in the constructor
+  private int deliveredCount;
+  private final List<String> events = new ArrayList<>();
+
+  public ShipmentTracker(String carrier) {
+    this.carrier = carrier;
+  }
+
+  public void recordDelivery(String trackingNumber) {
+    deliveredCount++;
+    events.add(carrier + ":" + trackingNumber);
+  }
+}
+
+

The deliveredCount field is incremented in recordDelivery, so it is genuinely mutable and no issue is raised on it.

+

Compliant solution

+
+public class ShipmentTracker {
+
+  private final String carrier;
+  private int deliveredCount;
+  private final List<String> events = new ArrayList<>();
+
+  public ShipmentTracker(String carrier) {
+    this.carrier = carrier;
+  }
+
+  public void recordDelivery(String trackingNumber) {
+    deliveredCount++;
+    events.add(carrier + ":" + trackingNumber);
+  }
+}
+
+

Resources

+

Documentation

+ +

Related rules

+
    +
  • {rule:java:S1170} - Public constants and fields initialized at declaration should be "static final" rather than merely "final"
    • +
  • {rule:java:S1450} - Private fields only used as local variables in methods should become local variables
    • +
  • {rule:java:S6206} - Records should be used instead of ordinary classes when representing immutable data structure
    • +
+

{rule:java:S1170} covers fields that are also constant and should become static final, whereas this rule covers per-instance values +that differ between objects. {rule:java:S1450} applies when a private field is used in a single method and should not be a field at all. +{rule:java:S6206} is the next step once every field of a data carrier is final, replacing the class with a record.

+ diff --git a/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9438.json b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9438.json new file mode 100644 index 00000000000..4e368b7a26e --- /dev/null +++ b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9438.json @@ -0,0 +1,24 @@ +{ + "title": "Private fields that are never reassigned after construction should be declared \"final\"", + "type": "CODE_SMELL", + "code": { + "impacts": { + "MAINTAINABILITY": "MEDIUM", + "RELIABILITY": "LOW" + }, + "attribute": "CLEAR" + }, + "status": "ready", + "remediation": { + "func": "Constant\/Issue", + "constantCost": "2min" + }, + "tags": [ + "design" + ], + "defaultSeverity": "Minor", + "ruleSpecification": "RSPEC-9438", + "sqKey": "S9438", + "scope": "Main", + "quickfix": "targeted" +} diff --git a/sonar-java-plugin/src/main/resources/profiles/Sonar_way/S9438 b/sonar-java-plugin/src/main/resources/profiles/Sonar_way/S9438 new file mode 100644 index 00000000000..e69de29bb2d