Further enhancing talawa_lint

[literalEval]

Is your feature request related to a problem? Please describe.
Talawa Lint is working great and contributors are getting hold of the new lint rules, but some things can be changed.

1. Exclude some files like test_helpers.dart, test_helper_mocks.dart etc, which are causing issues to contributors as no one wants to document such huge files and neither does it make sense. Alternatively, we can make another issue for documenting these file to a certain level.
2. Exclude some standard fields from doc check. Functions like main, test etc.
3. The params: and returns: blocks should have at least one blank line between them, for them to be shown correctly when user hovers over the corresponding field in the IDE. Although this is an issue related to the IDE itself, still we can fix this on our side. Having a blank line also improves readability.
4. talawa_good_doc_comments seems to have trouble with implicit return types. We need to fix that.

Chores

• Exclude test helper files from lint check
• Exclude standard field from doc check
• Treat setters and getters as normal fields.
• Enforce blank line between params: and returns: blocks.
• Make params: and returns: directives bold.
• Either enforce explicit return types, which is a better solution for readability, or enhance talawa_good_doc_comments to support implicit ones.
• Enforce a tab spaced None in params: block (currently the tab isn't enforced)
  02/04/2023
• Write a quick_fix template that will insert a doc template automatically.
  02/04/2023
• Add hyperlink to Talawa Docs in the error itself.
  07/04/2023

Bugs/Enhancements reported

• Documenting params in wrong order should give a descriptive message.
  Reported by @Ayush0Chaudhary , 07/04/23

[literalEval]

@palisadoes please confirm that this is an issue so that I can start working on it.

[literalEval]

@palisadoes I am excluding whole test folder from our custom lint.

Let me explain:

1. Test files mostly have a main function and a custom function that returns a widget, and both of these are self explanatory to everyone. Doesn't need any documentation.
2. But what about adding comments to tests so that others can understand what the test is testing ? Sure, but we are not checking these comments in talawa_lint anyway, and neither can it be checked. Because we can't judge which line needs a comment and which doesn't. The volunteers and mentors will need to check this.
3. If we don't do this, each test file will contain comments like these

/// This is main function.
void main() {}

/// This returns MyWidget for testing.
/// 
/// params:
/// None
/// 
/// returns:
/// * `MyWidget`: MyWidget
MyWidget getMyWidget() {
  return MyWidget();
}

[literalEval]

Please confirm so that I can open the corresponding PR. Thanks.

[palisadoes]

OK

[github-actions]

This issue did not get any activity in the past 10 days and will be closed in 365 days if no update occurs. Please check if the develop branch has fixed it and report again or close the issue.

[palisadoes]

@literalEval Is there any more work that needs to be done on this?

[literalEval]

yeah @palisadoes . Was busy writing proposal. Will do this by this weekend. Sorry for the delay.

[literalEval]

@palisadoes if a function has no parameters and no return value (void return type), should we make the params: and returns blocks optional ?

[palisadoes]

That would mean that no one would document the code if this were optional.
Are there any other ways to handle these cases?

[literalEval]

Contributors will still NEED to write documentation for it. That part is mandatory. The only optional part will be the params and returns blocks, that too when the function has both a void return type as well as empty parameter list. The point I am trying to make is that, say there is a function

void foo() {
    ....stuff...
}

In this case the doc looks like

/// Function that does stuff.
///
/// **params**:
///   None
///
/// **returns**:
///   None
void foo() {
    ....stuff...
}

As you can see, the params and returns blocks are taking unnecessary space and adding nothing to the documentation.

After the changes that I am proposing, it will look like

/// Function that does stuff.
void foo() {
    ....stuff...
}

[palisadoes]

The issue is that because it's optional people will just put a single line for everything.

So the overall quality of the documentation is now suffering because of a minority of functions and methods that require a little extra text.

This is not a good development.

[palisadoes]

All the work you have put in to fix the documentation via the linter will be circumvented by others when they realize they don't have to do this work.

[literalEval]

@palisadoes understood. Moreover, the original tasks of this issue are completed, but more will follow. So instead of closing this issue, let's keep it opened and add new tasks to it as they come. This will save us from opening unnecessary issues of the same kind. If you don't agree with this point, please feel free to close this :)

[literalEval]

@palisadoes when was the docs directory in the Talawa App codebase last updated ? And why are we storing in the Talawa Codebase in the first place? Because everything in the doc is already present in the codebase. Why store some extra unnecessary information?

[palisadoes]

It's used in push.yml to update the Talawa-Docs repo.

[literalEval]

That I understand. But we don't need to store these docs in our Talawa Codebase for us to update Talawa-Docs 👀

[palisadoes]

Open an issue to fix this

[literalEval]

Sure.

[literalEval]

@palisadoes can you think of anything extra that we can add to our lint system? Or should I halt it's further development for now and work on something else? :)

[Ayush0Chaudhary]

@literalEval did you fix all the wrong error handling ?

[literalEval]

@Ayush0Chaudhary see the PR just above. I have updated the error message. Please advise if it needs more updation,

[palisadoes]

What's happening with fixing the original linter package?

[literalEval]

@palisadoes I am having discussions with the plugin maintainer on Discord. The main issue at that time was writing tests for the changes. I think we can do that by collaborating with each other. Maybe I can write the changes and @Ayush0Chaudhary can write the tests?

The thing is that here in Talawa I can write code and if something goes wrong unexpectedly, I can simply write a patch fastly. But the said plugin is already in production and they need everything to be perfect in a single PR. The feature I am proposing there modifies their architecture to the core and thus needs very precise code.

[palisadoes]

That's a possibility, but you'll need to ask @Ayush0Chaudhary about his availability and willingness to do this.

Why hasn't there an update here? The tests he's suggesting don't look time consuming.

• Add support for linting a subset of files in the package invertase/dart_custom_lint#82

[Ayush0Chaudhary]

@palisadoes @literalEval @noman2002
I am glad you asked and will be happy to work with you @literalEval , but right now I am focusing on bringing back all broken features through #1748

I would need 4 Days to fix that issue

[literalEval]

@palisadoes the original PR was easy enough to write tests for. But then the author advised me to rewrite the implementation and then write tests accordingly. I am discussing the new implementation with him and will start working on it soon.

@Ayush0Chaudhary please let me know when you are free. Thanks.

[github-actions]

This issue did not get any activity in the past 10 days and will be closed in 365 days if no update occurs. Please check if the develop branch has fixed it and report again or close the issue.

[github-actions]

This issue did not get any activity in the past 180 days and thus has been closed. Please check if the newest release or develop branch has it fixed. Please, create a new issue if the issue is not fixed.

[literalEval]

@palisadoes please keep this opened. Though no bug has been found in our custom linter as of now, it will be good if this issue exists as a parent issue for any further enhancement in our linter. Thanks.

[github-actions]

This issue did not get any activity in the past 10 days and will be closed in 180 days if no update occurs. Please check if the develop branch has fixed it and report again or close the issue.

[github-actions]

This issue did not get any activity in the past 10 days and will be closed in 180 days if no update occurs. Please check if the develop branch has fixed it and report again or close the issue.

[palisadoes]

@literalEval

Are you still working on this?

[literalEval]

@palisadoes please keep this opened. Though no bug has been found in our custom linter as of now, it will be good if this issue exists as a parent issue for any further enhancement in our linter. Thanks.

This was the reason for keeping this issue open.