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

Fix puspec.yaml path when analysis_option.yaml is not at root #19

Closed
wants to merge 4 commits into from

Conversation

olexale
Copy link

@olexale olexale commented Jul 23, 2022

Thanks for this package! It makes the process of creating custom rules sooo much simpler, I love it! 💙

The package, though, does not work for cases when analysis_options.yaml is placed not near the pubspec.yaml file. This is the case for packages with custom linter settings where analysis_options is located somewhere inside lib folder. With this PR, I propose to introduce additional logic - when pubspec is not found, we may iterate through path components and try to find pubspec - it must be somewhere, right?

I've checked the package locally with the proposed changes, and it seems to work. Unfortunately, I was not able to write tests for these changes because almost all tests from the main branch are failing for me with

  FileSystemException: Cannot open file, path = '/Users/ole/dev/pets/dart_custom_lint/packages/example_lint/.dart_tool/package_config.json' (OS Error: No such file or directory, errno = 2)
  dart:io                            _File.readAsStringSync
  test/peer_project_meta.dart 43:40  new PeerProjectMeta.fromDirectory
  test/peer_project_meta.dart 56:42  PeerProjectMeta.current
  test/peer_project_meta.dart        PeerProjectMeta.current
  test/create_project.dart 36:29     createPlugin
  test/server_test.dart 169:20       main.<fn>
  test/server_test.dart 168:51       main.<fn>

Is there a missing package?

Anyway, happy to add tests if you agree that this functionality is useful.

return pubspecFile;
}

final pathComponents = p.split(packagePath);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of path manipulation, we can use File/Directory.parent

@rrousselGit
Copy link
Collaborator

Hello!
Sorry for the delay.

This needs some test. Fancy doing it?

@rrousselGit
Copy link
Collaborator

Hello! Do you still plan on working on this?

@rrousselGit rrousselGit added the question Further information is requested label Nov 23, 2022
@olexale
Copy link
Author

olexale commented Nov 23, 2022

Hey! Oh, I see that now tests are working on main. Sure, I'll update the PR soon.

@olexale
Copy link
Author

olexale commented Feb 15, 2023

Hello!

The proposed changes don't work with the latest version. After making the changes in both plugin.dart and server_to_client_channel.dart files, I see a RequestErrorCode.PLUGIN_ERROR: UnimplementedError error. I assume the linter is looking for a createPlugin implementation in the wrong folder for cases when analysis_options is not near pubspec.yaml.

I had problems with tests as dart run custom_lint completes without errors if I run it in the root of the package (near pubspec.yaml) and it will not run from the lib folder.

I'm closing the PR since it's outdated. I hope to revisit this issue later.

Thanks again for the plugin!
Regards

@olexale olexale closed this Feb 15, 2023
ghost pushed a commit to solid-software/dart_custom_lint that referenced this pull request Dec 5, 2023
* disable deprecated rule

* update changelog, version, description
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants