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

If a layer is allowed by :accessed-by-layers or :accesses-layers, it should not throw a violation #43

Open
thayannevls opened this issue Oct 1, 2023 · 1 comment

Comments

@thayannevls
Copy link

Hello! I'm developing two configurations using clj-depend, where I want one configuration know about layers of the another but not the other way around.

Imagine that this is configuration A:

:clj-depend {:layers {:foo {:defined-by "foo" :accessed-by-layers #{:one :two}}
                       ...}}

And this is configuration B:

:clj-depend {:layers {:bar {:defined-by "bar" :accesses-layers #{:foo}}
                       ...}}

I don't want that config A knows about B, so it's not a option for me to add :bar in accessed-by-layers in :foo.

The thing is clj-depend throws a violation even though I defined :accesses-layers #{:foo}. I believe this happens because of this code:

(defn- violate?
  [config
   {:keys [layer dependency-layer]}]
  (and (not= layer dependency-layer)
       (or (dependency-layer-cannot-be-accessed-by-layer? config dependency-layer layer)
           (layer-cannot-access-dependency-layer? config layer dependency-layer))))

If you agree that it should not throw an error in this case, we could change to return true only if it's not accessible in any way:

(defn- violate?
  [config
   {:keys [layer dependency-layer]}]
  (and (not= layer dependency-layer)
       (and (dependency-layer-cannot-be-accessed-by-layer? config dependency-layer layer)
            (layer-cannot-access-dependency-layer? config layer dependency-layer))))
@fabiodomingues
Copy link
Owner

I was analyzing your case, and I even wrote a test scenario to simulate this.

(deftest analyze-test-using-access-layers-and-accessed-by-layers
  (testing "should return zero violations when there is a layer defined that is not accessed by any other but there is a layer that defines which accesses it"
    (is (= []
           (analyzer/analyze {:config                    {:layers {:a {:defined-by         ".*\\.a\\..*"
                                                                       :accessed-by-layers #{}}
                                                                   :b {:defined-by         ".*\\.b\\..*"
                                                                       :accessed-by-layers #{:a}}
                                                                   :c {:defined-by         ".*\\.c\\..*"
                                                                       :access-layers      #{:b}}}}
                              :dependencies-by-namespace {'foo.a.bar #{'foo.c.bar 'foo.b.bar}
                                                          'foo.b.bar #{'foo.b.baz}
                                                          'foo.b.baz #{}
                                                          'foo.c.bar #{'foo.b.bar}}})))))

BTW, when writing the test scenario it seemed contradictory to have a config that defines that a layer should not be accessed by any other when in the same config there is another layer defined that can access it.

Mixing :accessed-by-layers and :access-layers can lead to confusion. It seems to make sense for clj-depend to validate when this occurs and alert the user to only use one form. Wdyt?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants