Implement logging warnings for deprecated param usages in funcion call and fix inconsistent behaviour in warnings in deprecated record fields

compiler/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/semantics/analyzer/CodeAnalyzer.java

@@ -2232,32 +2232,59 @@ public void visit(BLangTableConstructorExpr tableConstructorExpr, AnalyzerData d
 
     @Override
     public void visit(BLangRecordLiteral recordLiteral, AnalyzerData data) {
-        List<RecordLiteralNode.RecordField> fields = recordLiteral.fields;
+        BType referredType = Types.getReferredType(recordLiteral.getBType());
+        boolean isRecord = referredType.tag == TypeTags.RECORD;
+        List<RecordLiteralNode.RecordField> recordLiteralFields = recordLiteral.fields;
 
-        for (RecordLiteralNode.RecordField field : fields) {
+        LinkedHashMap<String, BField> recordFields = null;
+        if (isRecord) {
+            recordFields = ((BRecordType) referredType).getFields();
+        }
+
+        for (RecordLiteralNode.RecordField field : recordLiteralFields) {
             if (field.isKeyValueField()) {
                 analyzeExpr(((BLangRecordKeyValueField) field).valueExpr, data);
+                reportIfDeprecatedUsage(((BLangRecordKeyValueField) field).key.fieldSymbol, recordLiteral,
+                        ((BLangRecordKeyValueField) field).pos);
             } else if (field.getKind() == NodeKind.SIMPLE_VARIABLE_REF) {
-                analyzeExpr((BLangRecordLiteral.BLangRecordVarNameField) field, data);
+                BLangRecordLiteral.BLangRecordVarNameField recField
+                        = (BLangRecordLiteral.BLangRecordVarNameField) field;
+                analyzeExpr(recField, data);
+
+                if (isRecord) {
+                    BField matchingField = recordFields.get(recField.symbol.getName().getValue());
+                    if (matchingField != null) {
+                        reportIfDeprecatedUsage(matchingField.symbol, recordLiteral, recField.pos);
+                    }
+                }
             } else {
-                analyzeExpr(((BLangRecordLiteral.BLangRecordSpreadOperatorField) field).expr, data);
+                BLangRecordLiteral.BLangRecordSpreadOperatorField spreadField
+                        = (BLangRecordLiteral.BLangRecordSpreadOperatorField) field;
+                analyzeExpr(spreadField.expr, data);
+
+                BType spreadFieldType = Types.getReferredType(spreadField.expr.getBType());
+                if (isRecord && spreadFieldType != null && spreadFieldType.tag == TypeTags.RECORD) {
+                    for (BField fieldEntry: ((BRecordType) spreadFieldType).getFields().values()) {
+                        BField matchingField = recordFields.get(fieldEntry.getName().getValue());
+                        if (matchingField != null) {
+                            reportIfDeprecatedUsage(matchingField.symbol, recordLiteral, spreadField.expr.pos);
+                        }
+                    }
+                }
             }
         }
 
         Set<Object> names = new HashSet<>();
         Set<Object> neverTypedKeys = new HashSet<>();
-        BType literalBType = recordLiteral.getBType();
-        BType type = Types.getReferredType(literalBType);
-        boolean isRecord = type.tag == TypeTags.RECORD;
-        boolean isOpenRecord = isRecord && !((BRecordType) type).sealed;
+        boolean isOpenRecord = isRecord && !((BRecordType) referredType).sealed;
 
         // A record type is inferred for a record literal even if the contextually expected type is a map, if the
         // mapping constructor expression has `readonly` fields.
         boolean isInferredRecordForMapCET = isRecord && recordLiteral.expectedType != null &&
                 recordLiteral.expectedType.tag == TypeTags.MAP;
 
         BLangRecordLiteral.BLangRecordSpreadOperatorField inclusiveTypeSpreadField = null;
-        for (RecordLiteralNode.RecordField field : fields) {
+        for (RecordLiteralNode.RecordField field : recordLiteralFields) {
 
             BLangExpression keyExpr;
 
@@ -2277,7 +2304,7 @@ public void visit(BLangRecordLiteral recordLiteral, AnalyzerData data) {
                     }
                     inclusiveTypeSpreadField = spreadOpField;
 
-                    if (fields.size() > 1) {
+                    if (recordLiteralFields.size() > 1) {
                         if (names.size() > 0) {
                             this.dlog.error(spreadOpExpr.pos,
                                             DiagnosticErrorCode.SPREAD_FIELD_MAY_DULPICATE_ALREADY_SPECIFIED_KEYS,
@@ -2320,7 +2347,7 @@ public void visit(BLangRecordLiteral recordLiteral, AnalyzerData data) {
                         if (bField.type.tag != TypeTags.NEVER) {
                             this.dlog.error(spreadOpExpr.pos,
                                     DiagnosticErrorCode.DUPLICATE_KEY_IN_RECORD_LITERAL_SPREAD_OP,
-                                    type.getKind().typeName(), fieldName, spreadOpField);
+                                    referredType.getKind().typeName(), fieldName, spreadOpField);
                         }
                         continue;
                     }
@@ -2365,7 +2392,8 @@ public void visit(BLangRecordLiteral recordLiteral, AnalyzerData data) {
                                 unescapedName, inclusiveTypeSpreadField);
                     }
 
-                    if (!isInferredRecordForMapCET && isOpenRecord && !((BRecordType) type).fields.containsKey(name)) {
+                    if (!isInferredRecordForMapCET && isOpenRecord
+                            && !((BRecordType) referredType).fields.containsKey(name)) {
                         dlog.error(keyExpr.pos, DiagnosticErrorCode.INVALID_RECORD_LITERAL_IDENTIFIER_KEY,
                                 unescapedName);
                     }
@@ -2388,7 +2416,7 @@ public void visit(BLangRecordLiteral recordLiteral, AnalyzerData data) {
         }
 
         if (isInferredRecordForMapCET) {
-            recordLiteral.expectedType = type;
+            recordLiteral.expectedType = referredType;
         }
     }
 
@@ -2464,12 +2492,7 @@ public void visit(BLangFieldBasedAccess.BLangNSPrefixedFieldBasedAccess nsPrefix
     private void analyzeFieldBasedAccessExpr(BLangFieldBasedAccess fieldAccessExpr, AnalyzerData data) {
         BLangExpression expr = fieldAccessExpr.expr;
         analyzeExpr(expr, data);
-        BSymbol symbol = fieldAccessExpr.symbol;
-        if (symbol != null && Symbols.isFlagOn(fieldAccessExpr.symbol.flags, Flags.DEPRECATED)) {
-            String deprecatedConstruct = generateDeprecatedConstructString(expr, fieldAccessExpr.field.toString(),
-                    symbol);
-            dlog.warning(fieldAccessExpr.pos, DiagnosticWarningCode.USAGE_OF_DEPRECATED_CONSTRUCT, deprecatedConstruct);
-        }
+        reportIfDeprecatedUsage(fieldAccessExpr.symbol, expr, fieldAccessExpr.pos);
     }
 
     @Override
@@ -2496,6 +2519,7 @@ public void visit(BLangInvocation invocationExpr, AnalyzerData data) {
                 logDeprecatedWarningForInvocation(invocationExpr);
             }
         }
+        analyzeInvocationParams(invocationExpr, data);
     }
 
     @Override
@@ -3875,6 +3899,83 @@ private boolean checkReturnValidityInTransaction(AnalyzerData data) {
                 && data.withinTransactionScope;
     }
 
+    private void analyzeInvocationParams(BLangInvocation iExpr, AnalyzerData data) {
+        if (iExpr.symbol == null) {
+            return;
+        }
+
+        BType invocableType = Types.getReferredType(iExpr.symbol.type);
+        BInvokableSymbol invokableSymbol = ((BInvokableSymbol) iExpr.symbol);
+        List<BVarSymbol> reqParamSymbols = invokableSymbol.params;
+        int parameterCountForPositionalArgs = ((BInvokableType) invocableType).getParameterTypes().size();
+
+        int visitedArgCount = 0;
+        for (BLangExpression expr : iExpr.argExprs) {
+            switch (expr.getKind()) {
+                case NAMED_ARGS_EXPR:
+                    reportIfDeprecatedUsage(((BLangNamedArgsExpression) expr).varSymbol, expr, expr.pos);
+                    visitedArgCount++;
+                    break;
+                case REST_ARGS_EXPR:
+                    if (visitedArgCount >= parameterCountForPositionalArgs) {
+                        reportIfDeprecatedUsage(invokableSymbol.restParam, expr, expr.pos);
+                        continue;
+                    }
+
+                    BLangExpression restExpr = ((BLangRestArgsExpression) expr).expr;
+                    if (restExpr.getKind() == NodeKind.LIST_CONSTRUCTOR_EXPR) {
+                        visitedArgCount = analyzeRestArgsAgainstReqParams((BLangListConstructorExpr) restExpr,
+                                visitedArgCount, reqParamSymbols, invokableSymbol.restParam);
+                        continue;
+                    }
+
+                    for (int i = visitedArgCount; i < parameterCountForPositionalArgs; i++) {
+                        reportIfDeprecatedUsage(reqParamSymbols.get(i), expr, expr.pos);
+                    }
+                    break;
+                default:    // positional args
+                    if (visitedArgCount < parameterCountForPositionalArgs) {
+                        BVarSymbol paramSymbol = reqParamSymbols.get(visitedArgCount);
+                        reportIfDeprecatedUsage(paramSymbol, expr, expr.pos);
+                        if (Symbols.isFlagOn(reqParamSymbols.get(visitedArgCount).flags, Flags.INCLUDED)) {
+                            analyzeExpr(expr, data);
+                        }
+                    } else {
+                        reportIfDeprecatedUsage(invokableSymbol.restParam, expr, expr.pos);
+                    }
+                    visitedArgCount++;
+            }
+        }
+    }
+
+    private int analyzeRestArgsAgainstReqParams(BLangListConstructorExpr listConstructorExpr, int visitedArgCount,
+                                                 List<BVarSymbol> reqParamSymbols, BVarSymbol restParamSymbol) {
+        for (BLangExpression expr : listConstructorExpr.exprs) {
+            if (visitedArgCount >= reqParamSymbols.size()) {
+                // Visiting args matching with the rest-param
+                reportIfDeprecatedUsage(restParamSymbol, expr, expr.pos);
+                continue;
+            }
+
+            if (expr.getKind() != NodeKind.LIST_CONSTRUCTOR_SPREAD_OP) {
+                reportIfDeprecatedUsage(reqParamSymbols.get(visitedArgCount++), expr, expr.pos);
+                continue;
+            }
+
+            BLangExpression innerExpr = ((BLangListConstructorSpreadOpExpr) expr).expr;
+            if (innerExpr.getKind() == NodeKind.LIST_CONSTRUCTOR_EXPR) {
+                visitedArgCount = analyzeRestArgsAgainstReqParams((BLangListConstructorExpr) innerExpr,
+                        visitedArgCount, reqParamSymbols, restParamSymbol);
+                continue;
+            }
+
+            for (int i = visitedArgCount; i < reqParamSymbols.size(); i++) {
+                reportIfDeprecatedUsage(reqParamSymbols.get(i), innerExpr, innerExpr.pos);
+            }
+        }
+        return visitedArgCount;
+    }
+
     private void validateModuleInitFunction(BLangFunction funcNode) {
         if (funcNode.attachedFunction || !Names.USER_DEFINED_INIT_SUFFIX.value.equals(funcNode.name.value)) {
             return;
@@ -3925,6 +4026,15 @@ private BType getErrorTypes(BType bType) {
         return errorType;
     }
 
+    private boolean reportIfDeprecatedUsage(BSymbol constructSymbol, BLangExpression expr, Location usagePos) {
+        if (constructSymbol != null && Symbols.isFlagOn(constructSymbol.flags, Flags.DEPRECATED)) {
+            dlog.warning(usagePos, DiagnosticWarningCode.USAGE_OF_DEPRECATED_CONSTRUCT,
+                    generateDeprecatedConstructString(expr, constructSymbol.name.getValue(), constructSymbol));
+            return true;
+        }
+        return false;
+    }
+
     /**
      * This class contains the state machines for a set of workers.
      */

tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/annotations/DeprecationAnnotationTest.java

@@ -67,7 +67,13 @@ public void testDeprecationAnnotation() {
         BAssertUtil.validateWarning(compileResult, i++, "usage of construct 'Object3.fieldOne' is deprecated", 194, 9);
         BAssertUtil.validateWarning(compileResult, i++, "usage of construct 'Object3.t' is deprecated", 195, 9);
         BAssertUtil.validateWarning(compileResult, i++, "usage of construct 'add1' is deprecated", 200, 13);
+        BAssertUtil.validateWarning(compileResult, i++, "usage of construct 'x' is deprecated", 200, 18);
+        BAssertUtil.validateWarning(compileResult, i++, "usage of construct 'y' is deprecated", 200, 21);
+        BAssertUtil.validateWarning(compileResult, i++, "usage of construct 'z' is deprecated", 200, 24);
         BAssertUtil.validateWarning(compileResult, i++, "usage of construct 'add2' is deprecated", 201, 13);
+        BAssertUtil.validateWarning(compileResult, i++, "usage of construct 'x' is deprecated", 201, 18);
+        BAssertUtil.validateWarning(compileResult, i++, "usage of construct 'y' is deprecated", 201, 21);
+        BAssertUtil.validateWarning(compileResult, i++, "usage of construct 'z' is deprecated", 201, 24);
         BAssertUtil.validateWarning(compileResult, i++, "usage of construct 'Object1' is deprecated", 202, 5);
         BAssertUtil.validateWarning(compileResult, i++, "usage of construct 'z' is deprecated", 213, 13);
         BAssertUtil.validateWarning(compileResult, i++, "usage of construct 'Foo' is deprecated", 216, 38);
@@ -86,20 +92,61 @@ public void testDeprecationAnnotation() {
         BAssertUtil.validateWarning(compileResult, i++, "usage of construct 'Person.name' is deprecated", 287, 16);
         BAssertUtil.validateWarning(compileResult, i++, "usage of construct 'Person.getName' is deprecated", 288, 16);
         BAssertUtil.validateWarning(compileResult, i++, "usage of construct 'myFunction' is deprecated", 298, 5);
+        BAssertUtil.validateWarning(compileResult, i++, "usage of construct 'Employee.name' is deprecated", 316, 26);
+        BAssertUtil.validateWarning(compileResult, i++, "usage of construct 'Employee.job' is deprecated", 316, 49);
+        BAssertUtil.validateWarning(compileResult, i++, "usage of construct 'Job.experience' is deprecated", 316, 68);
         BAssertUtil.validateWarning(compileResult, i++, "usage of construct 'Employee.name' is deprecated", 317, 9);
         BAssertUtil.validateWarning(compileResult, i++, "usage of construct 'Employee.job' is deprecated", 319, 9);
         BAssertUtil.validateWarning(compileResult, i++, "usage of construct 'Employee.job' is deprecated", 320, 9);
         BAssertUtil.validateWarning(compileResult, i++, "usage of construct 'Employee.job' is deprecated", 321, 9);
-        BAssertUtil.validateWarning(compileResult, i++, "usage of construct 'Job.experiance' is deprecated", 321, 9);
+        BAssertUtil.validateWarning(compileResult, i++, "usage of construct 'Job.experience' is deprecated", 321, 9);
+        BAssertUtil.validateWarning(compileResult, i++, "usage of construct 'Employee2.name' is deprecated", 343, 28);
+        BAssertUtil.validateWarning(compileResult, i++, "usage of construct 'Employee2.job' is deprecated", 343, 51);
+        BAssertUtil.validateWarning(compileResult, i++, "usage of construct 'Job.experience' is deprecated", 343, 70);
         BAssertUtil.validateWarning(compileResult, i++, "usage of construct 'Employee2.name' is deprecated", 344, 9);
         BAssertUtil.validateWarning(compileResult, i++, "usage of construct 'Employee2.job' is deprecated", 346, 9);
+        BAssertUtil.validateWarning(compileResult, i++, "usage of construct 'name' is deprecated", 354, 17);
         BAssertUtil.validateWarning(compileResult, i++, "usage of construct 'name' is deprecated", 357, 9);
+        BAssertUtil.validateWarning(compileResult, i++, "usage of construct 'name' is deprecated", 370, 17);
         BAssertUtil.validateWarning(compileResult, i++, "usage of construct 'name' is deprecated", 373, 9);
+        BAssertUtil.validateWarning(compileResult, i++, "usage of construct 'Employee5.address' is deprecated",
+                393, 28);
         BAssertUtil.validateWarning(compileResult, i++, "usage of construct 'Employee5.address' is deprecated",
                 394, 9);
         BAssertUtil.validateWarning(compileResult, i++, "usage of construct 'Employee5.address' is deprecated",
                 395, 9);
         BAssertUtil.validateWarning(compileResult, i++, "usage of construct 'line02' is deprecated", 395, 9);
+        BAssertUtil.validateWarning(compileResult, i++, "usage of construct 'Job.experience' is deprecated", 414, 9);
+        BAssertUtil.validateWarning(compileResult, i++, "usage of construct 'Job.experience' is deprecated", 420, 9);
+        BAssertUtil.validateWarning(compileResult, i++, "usage of construct 'Job.experience' is deprecated", 426, 12);
+        BAssertUtil.validateWarning(compileResult, i++, "usage of construct 'Company.name' is deprecated", 433, 9);
+        BAssertUtil.validateWarning(compileResult, i++, "usage of construct 'Company.city' is deprecated", 434, 9);
+        BAssertUtil.validateWarning(compileResult, i++, "usage of construct 'Company.country' is deprecated", 435, 12);
+        BAssertUtil.validateWarning(compileResult, i++, "usage of construct 'x2' is deprecated", 458, 15);
+        BAssertUtil.validateWarning(compileResult, i++, "usage of construct 'x2' is deprecated", 459, 15);
+        BAssertUtil.validateWarning(compileResult, i++, "usage of construct 'x2' is deprecated", 460, 20);
+        BAssertUtil.validateWarning(compileResult, i++, "usage of construct 'Company.city' is deprecated", 463, 9);
+        BAssertUtil.validateWarning(compileResult, i++, "usage of construct 'Company.country' is deprecated", 464, 9);
+        BAssertUtil.validateWarning(compileResult, i++, "usage of construct 'Company.name' is deprecated", 465, 9);
+        BAssertUtil.validateWarning(compileResult, i++, "usage of construct 'city' is deprecated", 470, 9);
+        BAssertUtil.validateWarning(compileResult, i++, "usage of construct 'country' is deprecated", 471, 9);
+        BAssertUtil.validateWarning(compileResult, i++, "usage of construct 'name' is deprecated", 472, 9);
+        BAssertUtil.validateWarning(compileResult, i++, "usage of construct 'b' is deprecated", 476, 9);
+        BAssertUtil.validateWarning(compileResult, i++, "usage of construct 'c' is deprecated", 477, 9);
+        BAssertUtil.validateWarning(compileResult, i++, "usage of construct 'c' is deprecated", 478, 9);
+        BAssertUtil.validateWarning(compileResult, i++, "usage of construct 'c' is deprecated", 479, 9);
+        BAssertUtil.validateWarning(compileResult, i++, "usage of construct 'y1' is deprecated", 482, 9);
+        BAssertUtil.validateWarning(compileResult, i++, "usage of construct 'y2' is deprecated", 483, 9);
+        BAssertUtil.validateWarning(compileResult, i++, "usage of construct 'y1' is deprecated", 486, 9);
+        BAssertUtil.validateWarning(compileResult, i++, "usage of construct 'y2' is deprecated", 487, 9);
+        BAssertUtil.validateWarning(compileResult, i++, "usage of construct 'age' is deprecated", 493, 13);
+        BAssertUtil.validateWarning(compileResult, i++, "usage of construct 'x2' is deprecated", 499, 11);
+        BAssertUtil.validateWarning(compileResult, i++, "usage of construct 'x2' is deprecated", 502, 13);
+        BAssertUtil.validateWarning(compileResult, i++, "usage of construct 'x2' is deprecated", 509, 13);
+        BAssertUtil.validateWarning(compileResult, i++, "usage of construct 'x2' is deprecated", 516, 16);
+        BAssertUtil.validateWarning(compileResult, i++, "usage of construct 'b' is deprecated", 523, 13);
+        BAssertUtil.validateWarning(compileResult, i++, "usage of construct 'c' is deprecated", 524, 13);
+        BAssertUtil.validateWarning(compileResult, i++, "usage of construct 'c' is deprecated", 525, 13);
         Assert.assertEquals(compileResult.getWarnCount(), i);
     }