Create a documentation comment linter that is compatible with DartDoc

[anwersayeed]

We recently implemented automatic updating of the docs.talawa.io with descriptions extracted from the comments in the Talawa code base. This creates a wide range of HTML pages that explain the methods, functions and classes present in each file.

There are limitations with this approach.

1. It relies on the code contributors to be diligent in their comments so that they can be extracted
2. The formatting of the comments needs to be consistent.

There are many ways to approach this, but there are some general standards that are being accepted. For example there is the simple python pydocstyle linter that ensures basic compliance and the presence of comments immediately below each class, method and function declaration. This is insufficient.

Google has taken this one step further. In their python guide they explain that every method, class and function should have a description, definitions of the arguments and definitions of the results.

The TSDoc project attempts to do the same with Typescript, and have created a linter to help enforce compliance.

We want to maintain the quality of our documentation by identifying a linter which has a way of creating HTML documentation.

Research a method to enforce documentation compliance for dart comments.

• Every class, method, and function must have documentation about it

Documentation should include:

• A brief description of the code
• Argument variables that are used to initialize the code with a description of what each argument does
• Variables that the code returns and a description of each variable

Note

1. We are currently using DartDoc for generating HTML and would GREATLY PREFER to see whether there is any way to use it to enforce these or very similar standards
2. For example linting to make sure that the comments comply with the Dart Documentation Comments standard would be acceptable.
3. You can refer to this pull request for work that is being done by the dart team [wip] Introduce DartDoc "Summary Line" lint dart-archive/linter#3820, but others may be doing something through GitHub actions or some other means

Describe the solution you'd like

1. Finding a linting package that can be used as part of our GitHub actions
2. Finding a way for it to be run on all files or on just one file at a time.

Describe alternatives you've considered

1. Writing script files. We feel that there must be an existing solution to this.

[github-actions]

Congratulations on making your first Issue! 🎊 If you haven't already, check out our Contributing Guidelines and Issue Reporting Guidelines to ensure that you are following our guidelines for contributing and making issues.

[literalEval]

@anwersayeed @palisadoes I am interested in working on this issue. I am not very proficient with linters and docs but I am willing to learn.

[palisadoes]

Are you proficient with python?

One approach would be to write a python script that reads files and makes sure there is an comment block that includes:

1. An overall description,
2. argument descriptions and,
3. a description of the values returned,

for each method class and function. The comment block would need to also need to be located where DartDoc can process it.

The documentationcheck.py file could then be updated to flag all files that don't comply. It currently just checks modified files in the PR to see whether there is at least one comment line. It does nothing more. This is insufficient.

With this approach we can slowly get the documentation improved.

This is the minimum requirement.

Still interested? It requires an understanding of Dart and python or a similar scripting language. You can also try to research other linting solutions.

[literalEval]

I do know python and have written quite some scripts for daily use.

So is it like that the script will run on each file, parse it and check whether the

• Comments are present
• At the right place
• In the right format

with string parsing techniques ?

[palisadoes]

Yes.

It would need to report:

1. The filename
2. The line number where the error was found
3. The method/class/function where it was found
4. There argument(s)/return value(s) that are not documented where applicable
5. The general description that may be missing

It would also need to:

1. do this whether presented with filename(s) or a directory(ies) as an argument
2. recursively check files in the directory

User python's argparse module to differentiate between filenames or the directories.

The script would need to show these results after processing all the data presented, not just after the first file is analyzed.

Also, ensure you:

1. Follow the Google python style guide with your code.
2. Format the code using python black
3. Run the pylint linter on the code

Black and pylint could also be added to our GitHub actions to improve the quality of our python helper scripts.

The next issue to be created would be to do the same Google style analysis on our python scripts as an additional GitHub action. This would be useful for all our repositories.

Please take the time to do a good job. This is an important thing to fix.

[literalEval]

For now I am looking at custom linting in Flutter and will try to make one from scratch if I feel it can work.
If custom linting doesn't work, I will look at the Python script option and try file parsing.

[literalEval]

@palisadoes @anwersayeed I wanted to ask what is our target for documentation ? Is it enough if public classes and their methods, attributes etc are documented ?

Also, I am thinking that we would run this script after all the current tests finish so that the code will be syntactically correct and formatted and so extracting information while parsing will be straightforward.

[literalEval]

@palisadoes @anwersayeed I have found this lint option that asks for documentation for 'all public classes and their members'. Will it do ?

[palisadoes]

@palisadoes @anwersayeed I have found this lint option that asks for documentation for 'all public classes and their members'. Will it do ?

1. This is a good first step. Can the linter be applied to a single file so that it can be used by our existing PR python script?
2. We still need the check for standardized comments in classes, functions and methods.

[literalEval]

@palisadoes

1. I will check and update.
2. It already works for all classes and their public members (both attributes and methods) along with non class things like normal functions, enums etc. The only limitation being that it only work for things which are public and won't check for documentation of private fields. Is it okay ?

[palisadoes]

1. @literalEval That should be OK. Let's see what you have. What language are you using to do the linting?
2. @noman2002 @TheHazeEffect You should review this issue and PR. It should greatly improve our code quality and documentation

[literalEval]

@palisadoes what I am saying is that there already exists a lint rule in Flutter which when turned on, will

1. Show warning for every public class and its methods/attributes which are not documented
2. Can be run from command line for any specific file and show an error if it doesn't find documentation.

I am asking is will this much of documentation enforcement do ? Or we need to strictly check for documentation of private fields too ?

[palisadoes]

What you showed only checks for the existence comments in the right location. It doesn't check for the content of the comments.

1. We can implement your suggestion as it's better than what we have now. Can you think of a way that it can be included in our PR workflow so that it checks all changed files? If so, you should create a separate issue for that.
2. We still need the basis of this issue to be implemented. If you are unable to do this, for whatever reason, we should assign this issue to someone else.

[literalEval]

@palisadoes Hmm. I have thought of two possibilites.

1. Write a Python script which will check for existence of documentation and its format. I have started writing this script and have some success in basic checks. Basically I am doing this by simple string parsing.
2. Use this custom lint plugin and implement this functionality as native dart feature which can even be integrated to IDEs (as it directly works with dart analyzer).

I am currently working on the 1st option and will let you know about the progress.

[palisadoes]

@literalEval Please remember the aim is to enforce compliance as a GitHub action.

[literalEval]

Hmm. I'll try to make a basic working prototype and update in 2-3 days. This issue will obviously take long to complete :)

[literalEval]

@palisadoes I think I have got what we needed.

• After careful consideration, I thought that writing a python script will be a quite bad idea. As either I will write a simple parser, which by design, won't be extensible and will take a lot of effort to be fail safe, or I'll end up writing a full blown parser from scratch, which makes no sense as there is already a linter available for Dart.
• The best solution (of course) will be to write an Abstract Syntax Tree (AST) parser, but that will take too much work and neither can a single person do that for a proper language.

Looking at the problem again -

1. A lint option that checks for documentation of public classes, methods etc already exists and we only need to extend it to private fields as well.
2. Since this is just a dart script, it can be run from command line.
3. If we are sure that documentation for every fields exists, a simple string parser script can be written in Python that checks if the given documentation is in right format or not. I think this is all we need. Isn't it ?

So I came up with a solution for problem 1 -

1. Since public_member_api_docs checked for public fields, it must be doing some checks to find if a field is public or not. I forked up the default dart linter and modified the public_member_api_docs rule. After removing the public field checks, the linter was giving errors for ALL fields if they are not documented.

For lib/views/main_screen.dart, which is like

the modified linter gives below errors.

Now it checks for documentation for ALL the fields. The only problem that remains is that it will also ask for documentation of class constructors etc. but I am sure I can further modify it to not do so.

Is this good progress and should I continue in the same direction ? As I think there can't be any better idea than using the official linter itself. Please share your review and also any suggestions if you have.

Edits:

1. I fixed the class constructor issue.
2. The linter also exposes the documentation/comment associated with each node/field. So I think we can check the format from here itself.

[palisadoes]

1. What does it need to pass? What format does the documentation need to have?
2. If you get it fixed we need to add it as an option for the dart linter and submit a PR in the necessary repository. This will mean it will only be available for the latest versions of flutter. Do you see that as an issue?
3. With your solution, will we need to have our own linting repo until the official Dart one or can we place it on our .github directory? Would it be one file or a set of libraries?

It's good to see progress on this

[palisadoes]

@TheHazeEffect , @Sagar2366, @noman2002 what do you think about this issue and the protected solution?

[literalEval]

@palisadoes

1. It needs the path to file or directory. I will write the checks so that it reports compliance with DartDoc guidelines.
2. We don't need to add this into lint options. We can run this as separate script just like we are doing with python scripts.
3. Yes. We will store the fork in our workflow. After removing unnecessary files from it which we don't need, it would be of around 1-2 mb I guess.

Also, is this an exhaustive list of what we need ?

[palisadoes]

This is not practical. Including a complete linter in the code base will add bloat that will have to maintain.

Surely there must be a simple way to:

1. Read a file
2. Find the method class and functions
3. Determine whether there are comments above these locations
4. Determine whether the very first comment has text that doesn't refer to the arguments
5. Determine whether the comments mention the arguments in a specified format that a regular expression could do.
6. Make sure there are lines between the comments for readability.

This should not be hard. You are not giving this enough thought.

[literalEval]

@palisadoes And how about I create a plugin (this modified linter) and for talawa we only refer to that plugin instead of adding the whole thing to the codebase ? That will be highly extensible as maybe in the future we need some other custom styling to our files. We will easily implement them with this method.

There is also a custom_lint package here that allows to write custom lint rules. I can also try that and report as that won't require creating anything. Just a single dart file will do. I can also show a demo first before making changes to talawa, if it feels bloat even after that, then we will settle for python script. How's that ?
Also, creating custom lints isn't a new thing for orgs. While finding solutions to this problem, I found out that many orgs maintain their own custom lints.

Sure it can be done using string parsing with some python script but using a real linter is the most fail safe solution. But still, if everyone finds the python script idea better, I'll try to write a python script.

[palisadoes]

For tiny functions, yes. But most functions are more complex.

Remember, the reason for having this requirement is that we will be automatically extracting this data with DartDoc and adding it the Talawa website so that people can understand what all code does without having to examine it. In other words it improve our reference materials.

[literalEval]

That I totally understand @palisadoes. I have other ideas but maybe we can discuss them when this is merged, as version-2 of this implementation :)

[literalEval]

Can the void return doc be like returns: None or having a separate line is a must ?

[palisadoes]

Yes, I was thinking python which uses "none"

[literalEval]

Sorry @palisadoes I asked two questions at once and can't understand which one you agreed on :)
So a separate line is a must and returns: None in a single line shouldn't be acceptable. Right ?

[palisadoes]

Yes.

• Returns void, with the same expected layout that the arguments have with the heading on a different line from the variable descriptions

[literalEval]

1. About the void return, isn't the single line syntax better ? As it takes less space. Otherwise we'll be wasting another line just to say None
2. Understood. I believe the result should be the name of the variable that is returned ? Right ? Something like this ?

/// returns:
///     `mAge:` Age of the owner.
int myAge() {
    int mAge = 10;
    return mAge;
}

Also, have you thought about the returns: being optional when the return type is void ?

Like the 2nd point ?

[literalEval]

OR

it should be

/// returns:
///     `int`: age of owner

?

[palisadoes]

Let's leave that decision up to the coder. As long as they do the documentation we'll be much better off than we are today.

[literalEval]

Okay. I'm going with the 2nd option for now.

[evasharma12]

Hey @literalEval, I want to confirm that the HTML code will be generated smoothly with the linter.

1. Install Dart Doc and then run the command dart doc .
2. Modify some file's documentation according to the new format and then generate the HTML docs using the above command.
3. Post the screenshot for the same.

[tasneemkoushar]

I think I wasn't able to convey my point clearly.

1. The said script will fail if that string IS present, in any of the changed files.
2. New files won't have that // ignore directive and would automatically pass from the script and will be checked for documentation.

About the 2nd point, sure, I will look into these parameters thoroughly and make the script accordingly.

@literalEval

1. As I can see that the PR is merged, Is your documentation linter for dart doc comments working for new PRs now? As I tried to test it by altering comments and making the syntax wrong, but no error was reported!

2. When will you update Contributing.md for new PRs?

3. Won't it be confusing for people to remove the lint suppression directive when making a pull request?

[literalEval]

@evasharma12 will do.

[literalEval]

@tasneemkoushar

1. Final integration is not done yet. The command that checks for documentation in the codebase is present there but is commented out. It will be uncommented as the final step. For now, two steps remain
   • Add a script that checks if ignore directive is present or not. If present, it will fail, otherwise pass. I have made this script and only its integration with workflow remains.
   • Uncomment the lines from workflow which check for custom lint rules.
2. In the final step.
3. I don't think so. Contributing.md will explicitly mention this point and the workflow errors will too. If you think otherwise, please elaborate any alternative. Also, as you can see our discussion (with @palisadoes ) a couple of comments ago, you will see that I was also working on adding a feature to custom_lint that will allow us to check individual/group of files. That PR is pending and I will also try to get that merged. This ignore suppression is just the alternative to that.

Hope I answered all of your queries. Thanks.

[literalEval]

@tasneemkoushar can you answer my queries on Slack ? You are the possible mentor of the idea I wanted to work on for GSoC period. Thanks.

[tasneemkoushar]

@tasneemkoushar can you answer my queries on Slack ? You are the possible mentor of the idea I wanted to work on for GSoC period. Thanks.

Sure, Write me with all your queries on slack, I will get back to you as soon as I get time.