Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bal build is showing compilation warnings for external modules #32985

Open
Mirage20 opened this issue Sep 30, 2021 · 7 comments · Fixed by #42627
Open

Bal build is showing compilation warnings for external modules #32985

Mirage20 opened this issue Sep 30, 2021 · 7 comments · Fixed by #42627
Assignees
Labels
Area/ProjectAPI Team/DevTools Ballerina Developer Tooling ( CLI, Test FW, Package Management, OpenAPI, APIDocs ) Type/Improvement
Milestone

Comments

@Mirage20
Copy link

Description:
$title. For example, if we use an external module that does not have documented fields, we get undocumented field warnings when we compile the original project. Since the developer doesn't have any control over these warnings, it is better to remove these warnings from getting printed in the current project.

Steps to reproduce:

Build a project with an external module that has compilation warnings.

Example code:

import ballerinax/covid19;
import ballerina/http;

service / on new http:Listener(8090) {
    resource function get .(http:Request req) returns json|error? {

        covid19:Client covid19Endpoint = check new ();
    }
}

This will produce the following warnings

Compiling source
        mirajabeysekara/sandbox12:0.1.0
ballerinax/covid19:1.0.0 [central.ballerina.io -> home repo]  100% [=======================================================================] 12/12 KB (0:00:00 / 0:00:00) 
        ballerinax/covid19:1.0.0 pulled from central successfully
WARNING [ballerinax/covid19/1.0.0::types.bal:(163:5,163:27)] undocumented field 'cases'
WARNING [ballerinax/covid19/1.0.0::types.bal:(164:5,164:28)] undocumented field 'deaths'
WARNING [ballerinax/covid19/1.0.0::types.bal:(165:5,165:31)] undocumented field 'recovered'
WARNING [ballerinax/covid19/1.0.0::types.bal:(367:5,367:18)] undocumented field 'cases'
WARNING [ballerinax/covid19/1.0.0::types.bal:(368:5,368:28)] undocumented field 'deaths'
WARNING [ballerinax/covid19/1.0.0::types.bal:(369:5,369:31)] undocumented field 'recovered'

Running Tests

        sandbox12

                No tests found


Generating executable
Language telemetry service is disabled.
        target/bin/sandbox12.jar

Affected Versions:
SL Beta3 RC4

@anupama-pathirage
Copy link
Contributor

@hasithaa Thoughts on this? I don't think it is correct to remove these warnings even though they are coming from a dependent module.

@hevayo
Copy link
Contributor

hevayo commented Oct 1, 2021

IMO this is not a bug since it was a conscious decision to show any errors or warnings coming from the compiler for dependent modules.

Possible improvement can be introduce a mode to the compiler for compiling dependencies where the compiler can skip checks which are not relevant to a module user. ie ( lint errors, documentation issues etc)

@Mirage20
Copy link
Author

Mirage20 commented Oct 1, 2021

IMO this is not a bug since it was a conscious decision to show any errors or warnings coming from the compiler for dependent modules.

Here, the developer can't take any action to fix the warning right?
Also, these warnings will disappear on the next builds because of the caching.

Possible improvement can be introduce a mode to the compiler for compiling dependencies where the compiler can skip checks which are not relevant to a module user. ie ( lint errors, documentation issues etc)

+1

Another approach is to give flags to disable warnings like -Wundocumented-field similar to C++, but I am not sure whether it is the correct approach for Ballerina.

@sameerajayasoma
Copy link
Contributor

Yes, this is not a bug. I am closing this issue. Please create a new issue with a proposal.

@MaryamZi
Copy link
Member

Reopening this issue since this came up once again in a language design call when discussing #37463 and it was suggested that we shouldn't be showing these warnings/errors to the user.

@MaryamZi MaryamZi reopened this Sep 21, 2022
@MaryamZi MaryamZi added Team/DevTools Ballerina Developer Tooling ( CLI, Test FW, Package Management, OpenAPI, APIDocs ) Type/Improvement labels Sep 21, 2022
@sameerajayasoma
Copy link
Contributor

Considering recent issues/warnings, I now believe that we should suppress compile-time warnings generated by package dependencies. We decided to allow all such warnings a few years ago, but the situation has changed now.

My proposal is to suppress such warnings by default and provide a compiler option to see them.

@Thevakumar-Luheerathan
Copy link
Member

Reopening this issue again, we need to suppress the errors from the dependent modules and show a common error instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area/ProjectAPI Team/DevTools Ballerina Developer Tooling ( CLI, Test FW, Package Management, OpenAPI, APIDocs ) Type/Improvement
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

7 participants